Introduce group rule to branch namer#6784
Conversation
| module Dependabot | ||
| class PullRequestCreator | ||
| class BranchNamer | ||
| class SoloStrategy |
There was a problem hiding this comment.
This object represents our current approach to naming. I don't think Solo necessarily captures that well, but it stands as a complement to a grouping. I'm open to this changing now or later 🙂
| # frozen_string_literal: true | ||
|
|
||
| module Dependabot | ||
| class GroupRule |
There was a problem hiding this comment.
I'm a little unclear on the interfact for a GroupRule. The rule portion signifies a policy-like approach, where criteria must be met. I'm open to this changing, or to refining the concept as we move forward.
| end | ||
|
|
||
| def new_branch_name | ||
| group_rule.name |
There was a problem hiding this comment.
We should probably expect a group rule name to be a humanized string, as we'll need to either:
- Validate it strictly as being a valid branch name in the config file
- Accept any string and 'slugify' it here replacing invalid characters/spaces etc
I'd tend to prefer the latter as while it might end up making sense to validate it in the config for other reasons, this component probably shouldn't trust its inputs?
There was a problem hiding this comment.
- Accept any string and 'slugify' it here replacing invalid characters/spaces etc
I'd tend to prefer the latter as while it might end up making sense to validate it in the config for other reasons, this component probably shouldn't trust its inputs?
That makes sense to me. I'm going to leave this as is for now, in its very naive state, so we can make decisions on this when we have more information.
brrygrdn
left a comment
brrygrdn
left a comment
There was a problem hiding this comment.
👍🏻 This makes sense to me, I think sinking the existing logic into a discrete class is a good call vs just adding more to BranchNamer 😅 - SoloStrategy makes sense name-wise, we can revisit it later as we figure out how the grouped vs non-grouped behaviours play out.
landongrindheim
force-pushed
the
introduce-group-rule-to-branch-namer
branch
from
42f73b8 to
b1145de
Compare
This is the first step toward introducing a grouping strategy.
This isn't used yet, but will be to determine the name of a branch.
This object is intended to be a signifier for a grouping of dependencies. Merge with group rule
landongrindheim
force-pushed
the
introduce-group-rule-to-branch-namer
branch
from
b1145de to
56911b0
Compare
2 participants
As we move toward grouping updates, we'll need an alternative approach to naming. This pull request represents the structure of this change. Its details have been omitted intentionally as I believe we'll have better information as we get closer to fully supporting that workflow.