Skip to content

Conversation

@Exelord
Copy link
Contributor

@Exelord Exelord commented Oct 26, 2017

Description

Documentation for payment webhooks

@Exelord Exelord requested a review from Azdaroth October 26, 2017 08:38
Copy link
Contributor

@StoneFrog StoneFrog left a comment

Choose a reason for hiding this comment

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

Would be good to update changelog as well.
I was also thinking about updating webhooks screenshot with the new ones, but since it's just an example, we are probably good there.

@Exelord
Copy link
Contributor Author

Exelord commented Oct 27, 2017

@StoneFrog Isn't something which should be updated after a new release?
Basically, the flow should look like:

  1. Merge new PRs
  2. Update changelog
  3. Release new version
  4. Publish

So the updating of changelog should be in another PR which should also trigger a new release. But, of course, if you wish I can update it. I just never meet with such a release flow.

@StoneFrog
Copy link
Contributor

@Exelord Ha, you're more experienced than I'm, so you might be right. Until now I updated changelog in the PR introducing changes and just instead of version/date I used #master. Sometimes changes are not published immediately and adding changelog in PR introducing them is an easy way to make sure it won't be overlooked. Then, when ready to publish just update #master to date/version or whatever.

@Exelord
Copy link
Contributor Author

Exelord commented Oct 27, 2017

Good. You are right it cannot be overlooked. That's why github shows you the difference between the last version and the latest commit:
screen shot 2017-10-27 at 18 58 50

There are also cool tools like automatic changelog generator which based on closed PRs and issues generate it for you: https://github.com/skywinder/github-changelog-generator

Basically, the master branch doesn't mean the release, as you already know, so we can merge there even a documentation which is for upcoming features. The only thing we have to care about, we have to just keep the releases fully documented, what means that right before/after the version we have to add the changelog and publish the doc on a website.

In fact, do not expect that some outside contributor will update the changelog in a PR :/ This is not his responsibility.


I will add a changelog as you mentioned but I definitely suggest to change a release flow in the future. As an example, you can look to some big projects eg. ember-cli or any other following schematic versioning pattern.

@Exelord
Copy link
Contributor Author

Exelord commented Oct 30, 2017

@StoneFrog which date should I use in the changelog If I don't know when the feature will be merged?

# Changelog

## 2017-XX-11

  * [api improvement] Add `payment` webhooks

## 2017-18-10

  * [api improvement] Add `change_overs` endpoint.

## 2017-12-10

  * [doc update] Add guides for webhooks' subscriptions

@StoneFrog
Copy link
Contributor

@Exelord
Cool, any improvement is welcome. Especially those that save time. If it's not being merged right away I'd just keep it under

# Changelog

## master

  * [api improvement] Add `payment` webhooks

for the time being

@Azdaroth
Copy link
Member

@Exelord

In fact, do not expect that some outside contributor will update the changelog in a PR :/ This is not his responsibility.

I disagree here, it is a responsibility of the contributor to update docs, changelog etc. It is the case e.g. in Rails and as far as I remember is or used to be in ember-cli-mirage.

It's just a matter of adding something under #master and then the person doing the release will handle it properly.

@Azdaroth Azdaroth merged commit 6fbbd98 into master Oct 30, 2017
@Azdaroth Azdaroth deleted the feature-payment-webhooks branch October 30, 2017 12:37
@Exelord
Copy link
Contributor Author

Exelord commented Oct 31, 2017

@Azdaroth OK, here are 3 random PR's from ember-cli-mirage :)
https://github.com/samselikoff/ember-cli-mirage/pull/1199/files
miragejs/ember-cli-mirage#1187
miragejs/ember-cli-mirage#1171

and from rails
https://github.com/rails/rails/pull/31005/files
https://github.com/rails/rails/pull/30995/files

Contributors don't care about changelog because PRs and releases are totally different things. The natural changelog for code is the history of commits and for the project is a changelog.

When you are releasing the project (or a new version) you are going through all merged changes and based on that build the release changelog. And this is a one-time operation (can be automated) which do not rely on users engagement. This is the responsibility of project maintainer.

You can not assure the contributor that his merged changes will be present in the next version. Especially in the case when you have multiple release branches.

According to docs, I totally agree.

@Azdaroth
Copy link
Member

@Exelord Hmm, so maybe it used to be :P

But I definitely remember updating Changelog in Rails and writing docs to ember-cli-mirage when I was contributing...

Ok, that makes sense, but on the other hand, I think all PRs have been submitted by us so far, so it's not like we are making it harder that way for anyone to contribute :P

@Exelord
Copy link
Contributor Author

Exelord commented Oct 31, 2017

But I definitely remember updating Changelog in Rails and writing docs to ember-cli-mirage when I was contributing...

Yes, docs are totally required if the changes touch some API or behaviors. 👍

And definitely yes, we didn't make it harder :) We just shouldn't expect it from outside collaborators. This is not something obvious for them :)

It would be nice if in the future we can cover and split the release flow from the development ;)

@Azdaroth
Copy link
Member

Azdaroth commented Nov 1, 2017

@Exelord I think it depends on the project, not sure if it would help in this one. Someone from our team would need to add a Changelog, so ideally it would be the author of PR.

But in the gems or addons I guess it would make more sense.

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