Skip to content

[adjust-pages.pl] Add upper bound version support to patch helper#8912

Merged
chalin merged 12 commits intoopen-telemetry:mainfrom
vitorvasc:scripts_adjust-pages_add-maxvers-to-patch-helper
Apr 22, 2026
Merged

[adjust-pages.pl] Add upper bound version support to patch helper#8912
chalin merged 12 commits intoopen-telemetry:mainfrom
vitorvasc:scripts_adjust-pages_add-maxvers-to-patch-helper

Conversation

@vitorvasc
Copy link
Copy Markdown
Member

@vitorvasc vitorvasc commented Jan 15, 2026

  • Allows disabling patches via version constraints without removing code.
  • Add optional 4th parameter $maxVers to applyPatchOrPrintMsgIf() function
  • Patch skips when submodule version > maxVers (inclusive upper bound)
  • Backward compatible with existing 3-parameter calls

@chalin @vitorvasc
feat(patch-helper): add max version parameter to applyPatchOrPrintMsg…
ac0fee8 
…If function

Signed-off-by: Vitor Vasconcellos <vitor.vasconcellos@mercadolivre.com>
@vitorvasc vitorvasc requested a review from a team as a code owner January 15, 2026 11:43
@otelbot-docs otelbot-docs Bot requested a review from a team January 15, 2026 11:43
@vitorvasc vitorvasc force-pushed the scripts_adjust-pages_add-maxvers-to-patch-helper branch from ac0fee8 to 6f43910 Compare January 15, 2026 11:44
@vitorvasc vitorvasc removed the request for review from a team January 15, 2026 11:45
@vitorvasc vitorvasc removed the sig:obi label Jan 15, 2026
tiffany76
tiffany76 approved these changes Jan 15, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@tiffany76 tiffany76 left a comment

Choose a reason for hiding this comment

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

RSLGTM.

chalin
chalin reviewed Jan 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

I'll take a look ASAIC.

@chalin chalin marked this pull request as draft January 17, 2026 14:55
@vitorvasc vitorvasc added the CI/infra Repo CI & infrastructure label Jan 20, 2026
@vitorvasc vitorvasc marked this pull request as ready for review February 23, 2026 20:38
Copilot AI review requested due to automatic review settings February 23, 2026 20:38
@otelbot-docs otelbot-docs Bot added the ready-to-be-merged This PR is ready to be merged by a maintainer label Feb 23, 2026
Copilot AI reviewed Feb 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the patch helper system in adjust-pages.pl by adding optional upper bound version support. The change allows patches to be automatically skipped when the submodule version exceeds a specified maximum, without requiring code removal. This is useful for patches that address issues only present in a specific version range.

Changes:

  • Extended applyPatchOrPrintMsgIf() function to accept an optional 4th parameter $maxVers for specifying version upper bounds
  • Added new version check logic that skips patches when submodule version exceeds maxVers
  • Updated two existing patch calls to use the new maxVers parameter, limiting them to version 1.53.0

Comment thread scripts/content-modules/adjust-pages.pl Outdated
@chalin
Copy link
Copy Markdown
Contributor

chalin commented Feb 23, 2026

@vitorvasc - btw, I haven't forgotten about this PR. I've just been feeling that we can do better. Here are some ideas.

  1. I think that we should implement a function that parses semver IDs (X.Y.Z-pre-release-tag) and git describe-tag IDs (of the form X.Y.Z-dd-gHASH), and does an version-ID-parts based comparison, rather than a string/lexical comparison. The comparison function should accept partial IDs too, such as X.Y.Z-dd. (Actually, maybe we only need to support git describe-tag IDs for now.)

  2. We should try to move towards a (close to) data-driven approach. That is, where we'd have an array of patch info (spec, minver, maxver, anonymous subroutine to hold the code that will apply the patch).

WDYT?

@vitorvasc
Merge remote-tracking branch 'origin/main' into scripts_adjust-pages_…
330739f 
…add-maxvers-to-patch-helper

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc vitorvasc force-pushed the scripts_adjust-pages_add-maxvers-to-patch-helper branch from 784bd22 to 330739f Compare March 20, 2026 14:54
@vitorvasc
feat(patch): add support for versioned patches and migration link upd…
9ef54e0 
…ates

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
Copy link
Copy Markdown
Member Author

vitorvasc commented Mar 20, 2026

@chalin, both ideas make sense and I agree with them! Just pushed a new commit with both implementations.

PS: I'm not familiar with Perl, so I had my fellow Claude help me out with that. Let me know if you see any issues or opportunities to improve the file in general!

@vitorvasc
feat(patch): enhance patch entry structure and improve version check …
bc3f0ea 
…logging

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
npm run fix:format
a53b2c0 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
npm run fix:format
3df6753 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
npm run fix:format
310cd3e 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix(patch): correct key assignment in gitmodules cache
08ab1af 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix(patch): add cSpell ignore for frontmatter in content-module-patch…
01e1eed 
…es.md

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
Merge branch 'main' into scripts_adjust-pages_add-maxvers-to-patch-he…
b9d65e9 
…lper
@vitorvasc vitorvasc marked this pull request as draft April 6, 2026 17:43
chalin
chalin reviewed Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Nice! LGTM. We can later read the patch info from a file.

Is the current patch data up-to-date?

@chalin chalin marked this pull request as ready for review April 22, 2026 21:22
chalin
chalin approved these changes Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Actually, this is good as is. I'll followup with suggestions or a PR later.

@chalin chalin merged commit f21b3ea into open-telemetry:main Apr 22, 2026
26 checks passed
@chalin
Copy link
Copy Markdown
Contributor

chalin commented Apr 22, 2026

Merging b/c I want to add a patch! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@chalin chalin chalin approved these changes

@tiffany76 tiffany76 tiffany76 approved these changes

Assignees

@chalin chalin

Labels

CI/infra Repo CI & infrastructure ready-to-be-merged This PR is ready to be merged by a maintainer

Projects

SIG Comms: PRs & Issues
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vitorvasc @chalin @tiffany76