-
Notifications
You must be signed in to change notification settings - Fork 566
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
Skip SDK upgrades that are pinned to a specific commit #6238
base: main
Are you sure you want to change the base?
Conversation
…sable updates when the dependency is pinned to a specific commit or digest. The final rule ensures no updates are applied if the dependency's version source URL is a commit SHA. Related to open-telemetry#6192
This also changes some unrelated formatting of the file, making the diff harder to read. Could you remove those unrelated formats ? |
@dmathieu I've maintained the fille structure and ensured formatting is consistent and easy to read I'm not sure if that fixed it. Also the renovate bot link; renovate-schema.js file is unable to load from my end not sure why but maybe it could be the problem. Please let me know if that changed anything, thanks |
…nd easy to read Relate to open-telemetry#6238
…mplified content for reference/comments for the config package Related to open-telemetry#6238
Co-authored-by: Damien Mathieu <[email protected]>
Co-authored-by: Damien Mathieu <[email protected]>
Co-authored-by: Damien Mathieu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left two nits.
renovate.json
Outdated
} | ||
}, | ||
{ | ||
"_description": "Any SDK upgrade that made it through here is a pin/digest. DIsable it", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"_description": "Any SDK upgrade that made it through here is a pin/digest. DIsable it", | |
"_description": "Any SDK upgrade that made it through here is a pin/digest. Disable it", |
renovate.json
Outdated
@@ -31,8 +27,18 @@ | |||
"groupName": "golang.org/x" | |||
}, | |||
{ | |||
"_description": "Ignore pin and digest upgrades. We only want published releases of the SDK to trigger upgrades", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"_description": "Ignore pin and digest upgrades. We only want published releases of the SDK to trigger upgrades", | |
"_description": "Ignore pin and digest upgrades. We only want published releases of the SDK to trigger upgrades", |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6238 +/- ##
=======================================
+ Coverage 66.9% 67.2% +0.2%
=======================================
Files 190 191 +1
Lines 12540 12637 +97
=======================================
+ Hits 8395 8494 +99
+ Misses 3855 3854 -1
+ Partials 290 289 -1 |
renovate.json
Outdated
"allowedVersions": "/^v[0-9]+\\.[0-9]+\\.[0-9]+/" | ||
"enabled": true, | ||
"ignore": { | ||
"matchUpdateTypes": ["pin", "digest"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a pin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it's a dependency set at an explicit version.
https://docs.renovatebot.com/dependency-pinning/
Pinning isn't something we can really do with Go. But just to be same, maybe we should actually remove that match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this PR to be tested on a fork.
@Victorsesan, can you test it on https://github.com/Victorsesan/opentelemetry-go-contrib?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pellared I would be happy to! sorry for messing it up , i'm trying but my PR keeps getting tested here instead can you please give me some pointers on how to test directly on my fork repo . thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmathieu Did we succeeded in fixing this issue i would love a feedback if there is anything else i need to do i will be happy to! Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They would be, yes. But in your screenshot, they appear to say the configuration file is invalid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmathieu Finally succeeded in making renovate work as expected after a long try and errors, i had to make some minor changes for it to pass the test which I'm not sure if its good or bad but i mentioned all the errors i faced with the solution and modifications i did in my recent commit which i will be happy if you can check it out and let me know if we are in the right track. I would love to succeed in working on this . Thank you all for your time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shows the config is valid. But it doesn't seem to validate that commit upgrades would be skipped, while releases would be bumped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hows the config is valid. But it doesn't seem to validate that commit upgrades would be skipped, while releases would be bumped
@dmathieu Sorry i missed that, is this last commit good i have added something to fix it. I was just worried with mend not accepting the package rules even when they are good sometimes which made me lost track validating the skip rule and rather focus on having renovate pass the test, but let me know if the few changes i made does it and it did also pass the test. thanks
Problems i found with mend.io -After runnig the first test, the errors indicated that certain options are invalid, specifically the _description and ignore fields in my packageRules which i had to change and remove. *Solution: I removed _description Fields as they were classified as not a valid options in the Renovate configuration. *Kept the ignore Field which was valid, but ensured that it is used correctly. In this case, it remains in the rule for go.opentelemetry.io/otel/** -I ran a second test which had another error. The ignore field in packageRules[4] was still causing issues. The ignore option was not valid in the context of packageRules. *Solution: I used instead an enabled field to control whether updates are applied.
…mit upgrades. This will ensure that any dependency that is not explicitly allowed will be skipped.
No description provided.