chore: add prototype/example code for adding dynamic entity tags #11
Conversation
| @@ -0,0 +1,130 @@ | |||
| /** | |||
There was a problem hiding this comment.
This is the main file. I'm sure the todo's aren't exhaustive, but I tried to cover as much ground as I could (though credit where it's due, a lot of them were added by Asher while we were doing the pair programming session)
| constructor(urlReader: UrlReader, options: ProcessorOptions) { | ||
| this.urlReader = urlReader; | ||
| this.options = options; | ||
| } | ||
|
|
||
| static fromConfig(readerConfig: Config, options: ProcessorSetupOptions) { |
There was a problem hiding this comment.
This is a pattern that a lot of the plugins use, where they basically give you access to the raw constructor if you really need it, but you can also call the static fromConfig method to set things up with more sensible defaults
| } | ||
| >; | ||
|
|
||
| export class DevcontainersProcessor implements CatalogProcessor { |
There was a problem hiding this comment.
The CatalogProcessor interface has five methods on it:
getProcessorName(required)readLocation(optional)preProcessEntity(optional)postProcessEntity(optional)validateEntityKind(optional)
preProcessEntity is mainly for adding additional data right before the entity actually gets processed for real
We're also allowed to add any number of other methods/properties, but for safety, I'm keeping them all private
| builder.addProcessor( | ||
| DevcontainersProcessor.fromConfig(env.config, { | ||
| logger: env.logger, | ||
| eraseTags: false, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
This is where the plugin gets wired up to the deployment
Parkreiner
force-pushed
the
devcontainers-tag
branch
from
30abe91 to
e3d3a20
Compare
|
|
||
| const isGithubComponent = cleanUrl.includes('github.com'); | ||
| if (!isGithubComponent) { | ||
| return this.eraseTag(entity, DEFAULT_TAG_NAME); |
There was a problem hiding this comment.
Also, do we need to worry about other providers?
There was a problem hiding this comment.
It's mainly a quick check to see if the component even looks vaguely right, before we start kicking off more computationally-intense async requests with the search API
We will need to worry about other providers eventually, but I think the details are going to depend on what Asher turns up with the URLReader, so I didn't want to add extra code for Gitlab/Bitbucket just yet
There was a problem hiding this comment.
@Parkreiner why do we erase the tag if it's not a Github component?
There was a problem hiding this comment.
Well, it's just a preemptive move. I guess I was erring on the side of kicking logic off to the eraseTags method whenever there isn't a match, and having it decide whether it should erase anything
It wouldn't be too bad to decentralize the logic, but doing it this way makes sure that the checks for whether an erase should happen all happen in one central method. The pre-process method doesn't need to know whether the tag actually gets erased – as long as it delegates to the other method, that method can choose what happens next
There was a problem hiding this comment.
Ooo, I like the centralization! I was more curious why we were erasing at all. I would assume that if it's not a GitHub component (or Bitbucket component, or GitLab component) we return the entity as-is.
| builder.addProcessor( | ||
| DevcontainersProcessor.fromConfig(env.config, { | ||
| logger: env.logger, | ||
| eraseTags: false, |
There was a problem hiding this comment.
Let's just double check with Ben about the default here. I'll add it to our check-in notes.
Closes #49
Changes made
Notes