Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(manager/regex)!: allow arbitrary regex groups for templates #12296

Merged
merged 13 commits into from Nov 4, 2021

Conversation

secustor
Copy link
Collaborator

@secustor secustor commented Oct 24, 2021

Changes:

This PR allows the usage of arbitrary capture groups inside of regex manager templates and further adds refactor which this allows.

BREAKING_CHANGE
Only regex managers using the combination matchStringStrategy are affected of this change!
Currently capture groups which are empty but still match the regex are ignored.
This is no longer the case!
Subsequent matchGroups will now overwrite previous ones, even if the later one is empty.

Context:

Fixes #11387

Related: #12279

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@secustor secustor force-pushed the allow_arbitrary_capture_groups branch from 7091412 to 1765280 Compare October 25, 2021 09:03
@rarkins rarkins added the breaking Breaking change, requires major version bump label Oct 28, 2021
@rarkins
Copy link
Collaborator

rarkins commented Oct 28, 2021

@secustor could you write the BREAKING CHANGE description a little clearer? We'll issue a new major release in the next few working days once GOPROXY changes are ready

@rarkins rarkins changed the title feat(manager/regex): allow arbitrary regex groups for templates feat(manager/regex)!: allow arbitrary regex groups for templates Oct 29, 2021
@rarkins
Copy link
Collaborator

rarkins commented Oct 31, 2021

@secustor can you resolve the merge conflict?

@secustor
Copy link
Collaborator Author

@rarkins Done.

@rarkins
Copy link
Collaborator

rarkins commented Oct 31, 2021

Is it possible to separate out the refactor from the breaking change? It's currently pretty hard to review due to the file changes.

@secustor
Copy link
Collaborator Author

I have reverted the addition of an utils.ts, which should make the changes clearer.

But I'm not sure what is missing for codecov. In my overview there is no reference to the offending lines

@viceice
Copy link
Member

viceice commented Nov 1, 2021

I have reverted the addition of an utils.ts, which should make the changes clearer.

The idea was to open a new pr with just the refactoring, so we can merge that before this.

But I'm not sure what is missing for codecov. In my overview there is no reference to the offending lines

I think this is only a codecov issue, as there are no coverage changes. 🤷‍♂️

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to change our own regex managers or will they work as before?

Looks good so far

lib/manager/regex/index.ts Outdated Show resolved Hide resolved
@secustor
Copy link
Collaborator Author

secustor commented Nov 1, 2021

I have reverted the addition of an utils.ts, which should make the changes clearer.

The idea was to open a new pr with just the refactoring, so we can merge that before this.

But I'm not sure what is missing for codecov. In my overview there is no reference to the offending lines

I think this is only a codecov issue, as there are no coverage changes. man_shrugging

I thought it would be easier to create a PR for this when the changes are released than vice versa.

If you are referring to https://github.com/renovatebot/.github/blob/main/default.json, then no changes are necessary as this only affects regex managers with matchStringStrategy combination.

viceice
viceice previously approved these changes Nov 1, 2021
@rarkins rarkins changed the base branch from main to v29 November 4, 2021 08:44
@rarkins rarkins dismissed viceice’s stale review November 4, 2021 08:44

The base branch was changed.

@rarkins rarkins merged commit 69f3f9c into renovatebot:v29 Nov 4, 2021
rarkins pushed a commit that referenced this pull request Nov 5, 2021
)

Allow the usage of arbitrary capture groups inside of regex manager templates and further adds refactor which this allows.

BREAKING_CHANGE
Only regex managers using the combination matchStringStrategy are affected of this change!
Currently capture groups which are empty but still match the regex are ignored.
This is no longer the case!
Subsequent matchGroups will now overwrite previous ones, even if the later one is empty.
rarkins pushed a commit that referenced this pull request Nov 5, 2021
)

Allow the usage of arbitrary capture groups inside of regex manager templates and further adds refactor which this allows.

BREAKING_CHANGE
Only regex managers using the combination matchStringStrategy are affected of this change!
Currently capture groups which are empty but still match the regex are ignored.
This is no longer the case!
Subsequent matchGroups will now overwrite previous ones, even if the later one is empty.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2021
@secustor secustor deleted the allow_arbitrary_capture_groups branch October 22, 2022 15:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking Breaking change, requires major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate regex manager combination strategy
4 participants