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

fix(manager/go): fix replace block regex #15547

Merged
merged 15 commits into from May 29, 2022
Merged

Conversation

PhilipAbed
Copy link
Collaborator

@PhilipAbed PhilipAbed commented May 11, 2022

Changes

changed the regex so it would support comment lines inside replace block

Context

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

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.

needs test and regex is bad, see codeql error

@PhilipAbed
Copy link
Collaborator Author

PhilipAbed commented May 16, 2022

  • The Replace is being changed just so we could run the Command go get -t ./...,
    i added another replace section in the tests, the tests won't fail anyway because it doesn't change the final result
    there's no test i can add that can test the private function unless we use some reflection which isn't best practice..

  • The regex replace\s*\((?:(?:\s*\/\/.*)?(?:[^)])?)+\)
    is meant to find replace keyword+ spaces + ( everything that is not end bracket[^)]+ except (?:\s*\/\/.*)? commented line )
    i read some documentation about this issue, but i'm not that familiar with regex, i'm not sure how to rewrite my regex in a better way, can you help?

@PhilipAbed PhilipAbed requested a review from viceice May 17, 2022 08:44
nabeelsaabna
nabeelsaabna previously approved these changes May 17, 2022
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.

Did you real run against a test repo with latest changes?

@PhilipAbed
Copy link
Collaborator Author

Yes, i ran it on this: https://github.com/StinkyLord/15369-go-get-failed

rarkins
rarkins previously approved these changes May 23, 2022
viceice
viceice previously approved these changes May 25, 2022
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.

Very hacky, but i don't have a beter solution. Everyone should prefer to use gomodNoMassage

@rarkins rarkins enabled auto-merge (squash) May 27, 2022 09:33
auto-merge was automatically disabled May 29, 2022 12:00

Head branch was pushed to by a user without write access

@PhilipAbed PhilipAbed dismissed stale reviews from viceice, rarkins, and nabeelsaabna via e30a946 May 29, 2022 12:00
@rarkins rarkins merged commit 13d0255 into renovatebot:main May 29, 2022
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 32.68.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

go.sum update fails if go.mod contains a comment with a version
6 participants