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

[chore] mark confignet as stable #7954

Conversation

codeboten
Copy link
Contributor

This contents of this module have not changed in over 2 years

This contents of this module have not changed in over 2 years

Signed-off-by: Alex Boten <aboten@lightstep.com>
@codeboten codeboten requested review from a team and jpkrohling June 21, 2023 19:34
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.39%. Comparing base (9553bfe) to head (770eaf1).
Report is 52 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7954      +/-   ##
==========================================
+ Coverage   90.34%   90.39%   +0.04%     
==========================================
  Files         346      346              
  Lines       18194    18194              
==========================================
+ Hits        16438    16446       +8     
+ Misses       1422     1416       -6     
+ Partials      334      332       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bogdandrutu
Copy link
Member

Can you give me until mid July before marking any config stable? I want to also discuss somewhere and document things like:

  • Adding new validation rules means breaking change?
  • Should we offer a "NewDefaul...." for each config and say that the behavior of the config is stable only if initialized with NewDefault?
  • Adding new fields means breaking change? Let's assume someone "squash" the message into another message, then adding a field with same mapstruct value will be a breaking change, what do we do with that? What are the rules we follow.

There may be other things to document (just added some in my mind) but would like others to think of any special implications for configs that are not defined by the golang conventions of breaking compatibility,.

Copy link
Contributor Author

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

I think that's fine, i'll move this draft for now. My only motivation for moving this to RC is that there havent been changes in this dir for some time.

@bogdandrutu
Copy link
Member

@codeboten the time in my comment was to give us time to address my concerns, so I do believe someone (who is working full time, not like me until Sept) should address them and clarify what is possible and what is not in a stable config package.

@mx-psi
Copy link
Member

mx-psi commented Jun 29, 2023

Filed #8002 to discuss the stability guarantees for configuration structures. One of these was already discussed on the VERSIONING.md document (adding new fields was deemed to be fine).

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 14, 2023
@github-actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jul 29, 2023
@codeboten
Copy link
Contributor Author

Reopening this PR, with the questions answered in #8812

@codeboten codeboten reopened this Dec 7, 2023
@codeboten codeboten removed the Stale label Dec 7, 2023
@codeboten codeboten mentioned this pull request Dec 7, 2023
8 tasks
@bogdandrutu
Copy link
Member

One thing that we need to agree on, is that we should suffix everything with Config probably (instead of Settings) as discussed in one of the issues.

@bogdandrutu
Copy link
Member

Another thing is to make sure we pass "context" everywhere. Maybe TelemetrySettings as well

Copy link
Contributor

github-actions bot commented Jan 6, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added Stale and removed Stale labels Jan 6, 2024
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jan 25, 2024
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 23, 2024
Copy link
Contributor

github-actions bot commented Mar 9, 2024

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants