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

Prepare v1.8.3 release #2018

Merged
merged 8 commits into from
Mar 7, 2023
Merged

Conversation

ThomsonTan
Copy link
Contributor

Fixes #2007

Changes

Without including #2000 and #2005, this release is considered as a patch release.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@ThomsonTan ThomsonTan requested a review from a team March 6, 2023 21:12
@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #2018 (89d4e02) into main (04a0751) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2018   +/-   ##
=======================================
  Coverage   87.32%   87.32%           
=======================================
  Files         166      166           
  Lines        4723     4723           
=======================================
  Hits         4124     4124           
  Misses        599      599           

@marcalff
Copy link
Member

marcalff commented Mar 6, 2023

clang-format failed.

Please reformat the code located inside the pre-commit script:

const int major_version = ${major};
const int minor_version = ${minor};
const int patch_version = ${patch};
const char* pre_release = "${pre_release}";
const char* build_metadata = "${build_metadata}";
const int count_new_commits = ${count_new_commits};
const char* branch = "${branch}";
const char* commit_hash = "${latest_commit_hash}";
const char* short_version = "${short_version}";
const char* full_version =
    "${full_version}";
const char* build_date = "$(date -u)";

so that the generated code is correctly formatted by default, removing the need to apply clang-format by hand after generation.

@ThomsonTan
Copy link
Contributor Author

clang-format failed.

Please reformat the code located inside the pre-commit script:

const int major_version = ${major};
const int minor_version = ${minor};
const int patch_version = ${patch};
const char* pre_release = "${pre_release}";
const char* build_metadata = "${build_metadata}";
const int count_new_commits = ${count_new_commits};
const char* branch = "${branch}";
const char* commit_hash = "${latest_commit_hash}";
const char* short_version = "${short_version}";
const char* full_version =
    "${full_version}";
const char* build_date = "$(date -u)";

so that the generated code is correctly formatted by default, removing the need to apply clang-format by hand after generation.

Updated the pre-commit scripts accordingly.

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

There are a few PR ready to be merged:

Should we wait for them so they get included in the release ?

It will mean to run the pre commit script again.

Also, this release contains windows DLL and the opentracing shim,
so I think it should be named 1.9.0 anyway.

For PR #2000 and #2005, I assume it will take some time to review, due to size.

No strong opinion on wait versus release now.

@ThomsonTan
Copy link
Contributor Author

There are a few PR ready to be merged:

Should we wait for them so they get included in the release ?

It will mean to run the pre commit script again.

Also, this release contains windows DLL and the opentracing shim, so I think it should be named 1.9.0 anyway.

For PR #2000 and #2005, I assume it will take some time to review, due to size.

No strong opinion on wait versus release now.

For Windows DLL, it is a new feature and doesn't change any existing behavior, so I think keep the minor version is fine. For opentracing shim, probably the same applies? @marcalff @lalitb @esigo @owent how do you think?

For the listed small PRs, #2025 #2021 #2022, I suggest we don't merge them before merging this PR, unless it is necessary. So they can get more time in the repo to stabilize, and avoid risks on last-minute changes.

@esigo
Copy link
Member

esigo commented Mar 7, 2023

There are a few PR ready to be merged:

Should we wait for them so they get included in the release ?

It will mean to run the pre commit script again.

Also, this release contains windows DLL and the opentracing shim, so I think it should be named 1.9.0 anyway.

For PR #2000 and #2005, I assume it will take some time to review, due to size.

No strong opinion on wait versus release now.

For Windows DLL, it is a new feature and doesn't change any existing behavior, so I think keep the minor version is fine. For opentracing shim, probably the same applies? @marcalff @lalitb @esigo @owent how do you think?

For the listed small PRs, #2025 #2021 #2022, I suggest we don't merge them before merging this PR, unless it is necessary. So they can get more time in the repo to stabilize, and avoid risks on last-minute changes.

I'd say let's not rush the remaining PRs. Patch release would be fine IMO.
Thanks

@ThomsonTan
Copy link
Contributor Author

There are a few PR ready to be merged:

Should we wait for them so they get included in the release ?
It will mean to run the pre commit script again.
Also, this release contains windows DLL and the opentracing shim, so I think it should be named 1.9.0 anyway.
For PR #2000 and #2005, I assume it will take some time to review, due to size.
No strong opinion on wait versus release now.

For Windows DLL, it is a new feature and doesn't change any existing behavior, so I think keep the minor version is fine. For opentracing shim, probably the same applies? @marcalff @lalitb @esigo @owent how do you think?
For the listed small PRs, #2025 #2021 #2022, I suggest we don't merge them before merging this PR, unless it is necessary. So they can get more time in the repo to stabilize, and avoid risks on last-minute changes.

I'd say let's not rush the remaining PRs. Patch release would be fine IMO. Thanks

Seems #2022 has been merged. Then we can take the chance to merge the other 2 small PRs because we need to re-run the pre-commit script anyway.

@ThomsonTan
Copy link
Contributor Author

All 3 PRs listed above were merged. I am going to run pre-commit here and update.

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM.

Nit - release date in CHANGELOG is [1.8.3] 2023-03-06, should be [1.8.3] 2023-03-07

@ThomsonTan ThomsonTan merged commit f702676 into open-telemetry:main Mar 7, 2023
@owent
Copy link
Member

owent commented Mar 8, 2023

they

Agree not to merge these new features.We can include them in 1.9.0 later.

@ThomsonTan
Copy link
Contributor Author

they

Agree not to merge these new features.We can include them in 1.9.0 later.

Apologize for the confusion. As one of the smaller PR had already been merged when I made the request, I proceeded to merge all remaining smaller PRs and updated this release accordingly.

I will be more careful and send a notice to freeze the merge before our next release.

@ThomsonTan ThomsonTan deleted the pre_release_1.8.3 branch March 13, 2023 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prepare release v.1.9.0 (tentative: 3rd March)
5 participants