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

Release for 1.0.0-rc1 #824

Merged
merged 5 commits into from
Jun 4, 2021
Merged

Conversation

lalitb
Copy link
Member

@lalitb lalitb commented Jun 4, 2021

Release 1.0.0-rc1

Changes
First candidate release: 1.0.0-rc1

Milestone: https://github.com/open-telemetry/opentelemetry-cpp/milestone/8

1.0.0-rc.1     100% complete

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

@lalitb lalitb requested a review from a team June 4, 2021 13:59
@codecov
Copy link

codecov bot commented Jun 4, 2021

Codecov Report

Merging #824 (95406f4) into main (857fd31) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #824   +/-   ##
=======================================
  Coverage   95.52%   95.52%           
=======================================
  Files         156      156           
  Lines        6618     6618           
=======================================
  Hits         6321     6321           
  Misses        297      297           

Copy link
Contributor

@maxgolov maxgolov left a comment

Choose a reason for hiding this comment

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

I'd like to get #771 included. Without that change you will not get the promised ABI compatibility on Windows across different Visual Studio versions.

Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM.

@reyang
Copy link
Member

reyang commented Jun 4, 2021

I'd like to get #771 included. Without that change you will not get the promised ABI compatibility on Windows across different Visual Studio versions.

Probably not a blocker for RC?

@maxgolov
Copy link
Contributor

maxgolov commented Jun 4, 2021

Probably not a blocker for RC?

@reyang

I believe it's a blocker for anything to be labeled v1.0.0. For C++ - we are committed to be able to provide one build that is compatible with various runtimes. This is described in our previous agreed upon policy here.

We already have customers using the SDK / API with Visual Studio 2015. Example here:
#808

We cannot provide any customer the ABI guarantee on Windows until we merge the absl::variant PR as the default build option, because:

  • API surface without this PR is not accepting const char * parameters from initializer list, which I believe a major adoption blocker. All these parameters get wrongly treated as bool type, creating significant customer confusion.
  • same concern applies to Resource SDK part. None of the examples shown would work, and unit tests related Resource are gonna fail.

I'd summarize this as API surface for accepting string types in Visual Studio 2015 is broken and unusuable until the absl::variant PR is merged.

That is why I would like to propose the following options:

  • we either merge the absl::variant PR, that solves at least 3 different concerns (ABI, usability on vs2015, removal of unnecessary Boost license code from API headers).
  • or we label this release v0.8.0 , because it has not reached the quality bar for v1.0.0

CHANGELOG.md Show resolved Hide resolved
@lalitb
Copy link
Member Author

lalitb commented Jun 4, 2021

We cannot provide any customer the ABI guarantee on Windows until we merge the absl::variant PR as the default build option. Also the API surface without this PR is not accepting const char * parameters from initializer list, which I believe a major adoption blocker.

@maxgolov - We are feature complete for most of the otel components, have gone through sufficient cycles of alpha/beta release before moving to release candidate. And we don't guarantee ABI compatibility in the release candidate across all the platforms, it's a more complete release but things can still break. For now, will include #771 in rc1 once we can get approvals from others ( I have already reviewed it, looks good, and given my approval yesterday ). Will hold this release for now.

Copy link
Contributor

@maxgolov maxgolov left a comment

Choose a reason for hiding this comment

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

LGTM

@lalitb lalitb merged commit 93dd39b into open-telemetry:main Jun 4, 2021
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.

4 participants