Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Add a CHANGELOG file #251

Merged
merged 3 commits into from
Feb 21, 2018
Merged

Add a CHANGELOG file #251

merged 3 commits into from
Feb 21, 2018

Conversation

tedsuo
Copy link
Member

@tedsuo tedsuo commented Jan 16, 2018

@coveralls
Copy link

coveralls commented Jan 17, 2018

Coverage Status

Coverage remained the same at 81.515% when pulling a47aa44 on tedsuo/add-changelog into 242ba95 on master.

CHANGELOG.md Outdated
@@ -0,0 +1,13 @@
Changes by Version
==================
Copy link
Member

Choose a reason for hiding this comment

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

Could be used the same formatting symbols as in other markdown files in this repository. e.g. # for headers, * for lists

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to be uniform by copying the style in the go CHANGELOG ;) : https://raw.githubusercontent.com/opentracing/opentracing-go/master/CHANGELOG.md\

We also use .rst instead of .md in some repos. 🤷‍♂️

I prefer # and * so I'll switch to that. Maybe we can standardize and change the others as part of the upcoming "language maintainer" initiative.

CHANGELOG.md Outdated
==================

v0.31.0 (2018-01-12)
--------------------
Copy link
Member

Choose a reason for hiding this comment

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

Could we add one list item per added change? All the following would be in the one list item. Also, add a link to the PR, in some projects there is also a name of the commiter.

Two major changes are missing there:

  • introduction of log fields
  • removed span.log(object) methods

Copy link
Member Author

@tedsuo tedsuo Jan 17, 2018

Choose a reason for hiding this comment

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

I'm not sure what you mean by "All the following would be in the one list item." Do you mean only one list item for all the Scope/ActiveSpan changes?

Copy link
Member

Choose a reason for hiding this comment

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

Yes as it was part of one PR

Copy link
Member Author

Choose a reason for hiding this comment

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

I looked though the PRs... unfortunately I don't think the Pull Requests line up cleanly, as we went through many revisions. I will try to find a middle ground.

Copy link
Collaborator

@carlosalberto carlosalberto left a comment

Choose a reason for hiding this comment

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

Nice! Besides Pavol's remarks, I'd also mention that:

  • We have deprecated startManual() in favor of start().
  • Added an examples directory.
  • Maybeeee mention MockTracer uses TEXT_MAP as its default propagator, and added multi support reference.

@yurishkuro
Copy link
Member

should we adopt the http://keepachangelog.com/en/1.0.0/ format?

@tedsuo
Copy link
Member Author

tedsuo commented Jan 17, 2018

@yurishkuro http://keepachangelog.com/en/1.0.0/ seems nice but I'd like to get a language maintainer group together to start making these decisions. There's a set of things we should standardize on, such as PR/Issue templates, release procedures, etc...

@tedsuo
Copy link
Member Author

tedsuo commented Jan 30, 2018

@pavolloffay @carlosalberto @yurishkuro I updated this PR, sorry for the delay.

  • Switched to more standard markdown syntax.
  • The PRs in the v0.31 were too messy to reference, as a result of numerous revisions, so I did not include them.
  • We could include a link to the tracking issue, but I'm not sure how helpful that is (0.31.0 Release Candidate #189)
  • Instead I simplified the line items to objects which were added and removed from the core API, and method changes on the Tracer object. Hopefully that's the right level of detail.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

LGTM. Were there any other minor API changes, like in the tags?

@pavolloffay
Copy link
Member

LGTM, although at least one minor yet important thing is missing. Removed log.(Object) methods.

@pavolloffay
Copy link
Member

I will merge this and open subsequent PR to add more links in there.

@pavolloffay pavolloffay merged commit 3e270ab into master Feb 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants