dd-trace-rb icon indicating copy to clipboard operation
dd-trace-rb copied to clipboard

Raise minor version to 2.11.0

Open lloeki opened this issue 11 months ago • 9 comments

What does this PR do?

Raise minor version after 2.10.0 release.

Motivation:

  • master then targets 2.11.0
  • development version will properly order WRT 2.10.0, being 2.11.0.dev.b<git branch hash>.<ci provider><ci monotonic id>.g<git short sha>
  • system tests will be able to target master without conflicting with released 2.10.0

Change log entry

Nope.

Additional Notes:

How to test the change?

lloeki avatar Feb 06 '25 10:02 lloeki

Is the dev suffix added somewhere else? Should we add it here in the version.rb instead?

Currently version.rb defaults to the release version format (no PRE nor BUILD).

There's tooling to patch it in: https://github.com/DataDog/dd-trace-rb/blob/a444023a8f2a29250d46377e9010ab4fc2ed8623/.gitlab/patch_gem_version.sh

This is used by both GHA (when building dev gems that can be published to GitHub Packages) and GitLab CI (when building SSI images):

  • https://github.com/DataDog/dd-trace-rb/blob/a444023a8f2a29250d46377e9010ab4fc2ed8623/.github/workflows/build-gem.yml#L35-L48
  • https://github.com/DataDog/dd-trace-rb/blob/a444023a8f2a29250d46377e9010ab4fc2ed8623/.gitlab-ci.yml#L83-L86

With this change I am wondering whether we should:

  • default to a dev format in source (one that would be compatible with local dev, not just CI)
  • use the release format only when doing an actual release

But that's a question for another day I guess.

lloeki avatar Feb 06 '25 10:02 lloeki

I think it'd be nice to have the "dev" in the regular version.rb, rather than a weird patch but +1 on punting on this for later if it's opening a bit of a pandora's box of things that need to be fixed elsewhere :)

ivoanjo avatar Feb 06 '25 10:02 ivoanjo

Benchmarks

Benchmark execution time: 2025-02-06 12:05:02

Comparing candidate commit da8d44903cffecb15183d874f58b23ade04e5eb6 in PR branch lloeki/raise-dev-minor-version with baseline commit a444023a8f2a29250d46377e9010ab4fc2ed8623 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 31 metrics, 2 unstable metrics.

pr-commenter[bot] avatar Feb 06 '25 10:02 pr-commenter[bot]

Datadog Report

Branch report: lloeki/raise-dev-minor-version Commit report: da8d449 Test service: dd-trace-rb

:white_check_mark: 0 Failed, 22077 Passed, 1477 Skipped, 6m 4.6s Total Time

@cbeauchesne do you anticipate any issue with this change? i.e is there any ruby-specific off-by-one system test code that would be upset by us updating the version in master right after a release?

lloeki avatar Feb 06 '25 11:02 lloeki

BTW, this change in gemspec should propagates to all the lockfiles under gemfiles/*.

However, our automation does not kicked in. https://github.com/DataDog/dd-trace-rb/actions/runs/13176490545/attempts/1

TonyCTHsu avatar Feb 06 '25 11:02 TonyCTHsu

I triggered again: https://github.com/DataDog/dd-trace-rb/actions/runs/13176490545

TonyCTHsu avatar Feb 06 '25 11:02 TonyCTHsu

@lloeki the only ruby specific trick is here : https://github.com/DataDog/system-tests/blob/main/utils/_context/library_version.py#L113 -> No reason to have anything bad.

The second point, which is not ruby specific, is if somebody declared a feature working for 2.11.0, but the feature is not done/bugged. Your CI will catch such issue in this PR in theory. For scenario not included in your CI, well, you'll have a ping from myself after the merge.

cbeauchesne avatar Feb 06 '25 11:02 cbeauchesne

Thanks @cbeauchesne this is exactly the "off-by-one" I was looking for!

https://github.com/DataDog/system-tests/blob/65b9baeced2de4f3053d367f9d45332a33ecff65/utils/_context/library_version.py#L117-L118

            if library == "ruby":
                if len(self.version.build) != 0 or len(self.version.prerelease) != 0:
                    # we are not in a released version

                    # dd-trace-rb main branch expose a version equal to the last release, so hack it:
                    # * add 1 to minor version
                    # * and set z as prerelease if not prerelease is set, becasue z will be after any other prerelease

                    # if dd-trace-rb repo fix the underlying issue, we can remove this hack.
                    self.version = Version(
                        major=self.version.major,
                        minor=self.version.minor,
                        patch=self.version.patch + 1,
                        prerelease=self.version.prerelease,
                        build=self.version.build,
                    )

                    if not self.version.prerelease:
                        self.version = Version(
                            major=self.version.major,
                            minor=self.version.minor,
                            patch=self.version.patch,
                            prerelease=("z",),
                            build=self.version.build,
                        )

It looks like that starting with this PR the whole special casing should go away:

  • previously the semantics of ordering was 2.10.0 < 2.10.0.dev.whatever < 2.11.0 (which does not respect Ruby's way of ordering versions)
  • after this PR - the semantics of ordering becomes 2.10.0 < 2.11.0.dev.whatever < 2.11.0 (which does respect Ruby's way of ordering versions)

lloeki avatar Feb 06 '25 13:02 lloeki