Add archive registry support for curated plugin #2797
Conversation
| @@ -232,9 +235,10 @@ message RegistryConfig { | |||
| MavenConfig maven_config = 3; | |||
| SwiftConfig swift_config = 4; | |||
| PythonConfig python_config = 5; | |||
| ArtifactConfig artifact_config = 6; | |||
There was a problem hiding this comment.
I could see this going both ways, but I was thinking this would be called archive_config.
However, I could see a world where we want to add an archive_config to a plugin to modify it's behavior within the archive registry (for example, propagating a plugin option), but not necessarily show that plugin as part of the bundled artifact feature.
I wonder if the config should be named archive_config and have an "include in bundled artifacts" option. This would allow us to set some plugins to true and retain the flexibility to have ad-hoc archive_config without affecting the bundled artifact feature.
cc @pkwarren @stefanvanburen if you have some thoughts let us know.
There was a problem hiding this comment.
yeah, archive makes sense to me.
There was a problem hiding this comment.
chatted with Mike, renamed this to archive and we can start from there
| } | ||
| // Reserved for future remote registry types. | ||
| reserved 6 to 9; | ||
| reserved 7 to 9; |
There was a problem hiding this comment.
Just a point of order: this is sort of an abuse of the reserved keyword, reserved is really meant to say "these were previously used", outside of hyper-optimization theres no need to reserve these fields ahead of time.
There was a problem hiding this comment.
We definitely abused the use of reserved here to group all current and future configurations together, rather than for performance. In hindsight, this was probably us trying to be overly clever.
There was a problem hiding this comment.
You can still group them together in the source code, regardless of tag - tag is kind of irrelevant unless you're trying to save one byte (you're not)
cyinma
changed the title
No description provided.