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: introduce changelog generation #220

Merged
merged 5 commits into from
Oct 26, 2017
Merged

Conversation

TylorS
Copy link
Member

@TylorS TylorS commented Oct 26, 2017

No description provided.

@TylorS TylorS requested a review from Frikki October 26, 2017 18:01

### For generating changelog (for those with NPM publishing rights)

* [Ruby >= v2.1.0](https://www.ruby-lang.org/en/downloads/)
Copy link
Member

Choose a reason for hiding this comment

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

gem install rake is also required on Debian based systems such as Ubuntu and Linux Mint.

Copy link
Member

Choose a reason for hiding this comment

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

export CHANGELOG_GITHUB_TOKEN

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah these would be good to add 👍

@@ -30,9 +30,10 @@
"test:unit": "lerna run test:unit",
"test:lint": "lerna exec -- ../../node_modules/.bin/prettier --write --print-width 100 --tab-width 2 --no-semi --single-quote --trailing-comma es5 --parser typescript src/**/*.ts",
"build": "node tools/build.js",
"changelog": "github_changelog_generator --no-compare-link --bug-labels 'Type:\u0020Bug' --enhancement-labels 'Type:\u0020Feature,Type:\u0020Chore' --include-labels 'Type:\u0020Feature,Type:\u0020Chore,Type:\u0020Bug,Type:\u0020Breaking,Type:\u0020Regression' --user motorcyclets --project motorcycle",
Copy link
Member

Choose a reason for hiding this comment

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

why these \u0020 codes?

Copy link
Member Author

Choose a reason for hiding this comment

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

They are spaces, it doesn't seem to work with actual spaces in their place. It errors trying to read them.

Copy link
Member

Choose a reason for hiding this comment

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

I thought so.

Copy link
Member

@Frikki Frikki left a comment

Choose a reason for hiding this comment

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

Just a few ...

README.md Outdated

In your terminal, run the following:

NOTE: `sudo` may be required for all of the following terminal commands -- based
Copy link
Member

Choose a reason for hiding this comment

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

Remove:

-- based on where you installed Ruby.

It's just blah blah.

README.md Outdated
gem install github_changelog_generator
```

In your `.bash_profile`, `.zshrc` or equivalent shell configuration file add the
Copy link
Member

Choose a reason for hiding this comment

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

Comma after "configuration file".

README.md Outdated
export CHANGELOG_GITHUB_TOKEN=$GITHUB_PERSONAL_ACCESS_TOKEN_GENERATED_ABOVE
```

and save. In a new terminal you will now be able to successfully run `yarn changelog`
Copy link
Member

Choose a reason for hiding this comment

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

Remove "and save." We are not dealing with noob computer users who will generate change logs.

Correct:

In a new terminal, you should now be able to successfully run yarn changelog

"docs": "node tools/docs.js && git add **/README.md && git commit -m 'docs(README): rebuild documentation' && git push",
"release:pre": "yarn test && yarn build",
"release:post": "yarn docs",
"release:post": "yarn docs && yarn changelog && git add CHANGELOG.md && git commit -m 'docs(CHANGELOG): rebuild changelog && git push'",
Copy link
Member

Choose a reason for hiding this comment

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

Why this docs(CHANGELOG):? It seems so ... well ... unnecessary.

Copy link
Member Author

@TylorS TylorS Oct 26, 2017

Choose a reason for hiding this comment

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

Just following the convention still in the docs script. Should we look at removing that convention from both scripts in another issue/PR or here?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm! They need to be removed together. If we follow the one-change policy, then another issue/PR. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, another issue and PR.

Copy link
Member

@Frikki Frikki left a comment

Choose a reason for hiding this comment

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

Last bits.

README.md Outdated
export CHANGELOG_GITHUB_TOKEN=$GITHUB_PERSONAL_ACCESS_TOKEN_GENERATED_ABOVE
```

In a new terminal, you will now be able to successfully run `yarn changelog`
Copy link
Member

Choose a reason for hiding this comment

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

Not "will", but should. There's no guarantee.

README.md Outdated

In your terminal, run the following:

NOTE: `sudo` may be required for all of the following terminal commands.
Copy link
Member

Choose a reason for hiding this comment

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

Correction:

Note:

That is, **Note:**.

Copy link
Member

@Frikki Frikki left a comment

Choose a reason for hiding this comment

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

We're almost there :)

README.md Outdated

In your terminal, run the following:

**NOTE**: `sudo` may be required for all of the following terminal commands.
Copy link
Member

Choose a reason for hiding this comment

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

Almost ... **Note:**

@Frikki Frikki merged commit edcaec1 into master Oct 26, 2017
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.

2 participants