Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

Improvements to the project #111

Merged
merged 11 commits into from
Jul 26, 2017
Merged

Improvements to the project #111

merged 11 commits into from
Jul 26, 2017

Conversation

vadimdemedes
Copy link

@vadimdemedes vadimdemedes commented Jul 26, 2017

This PR brings multiple improvements to analytics-node as an open source project. It doesn't contain any changes to the code nor does it alter any behavior.

List of changes with short descriptions why:

1. Add editorconfig

Ensures that all contributors write code with the same formatting according to the standard linter. Requires editorconfig plugin to be installed in the user's editor, but experience shows most of them have it.

2. Remove History.md in favor of GitHub releases

As discussed with @stevenmiller888, GitHub releases are a more pleasant way to write informative releases, unlike the History.md, which is basically a copy of commit log. Example of what you can do with releases: https://github.com/avajs/ava/releases.

3. Remove yarn.lock and disable npm package locks (package-lock.json)

Those are ignored by both Yarn and npm anyway, so there's no reason to include them into repository. Package locks make sense in apps, not libraries. See sindresorhus/ama#479 (comment) for a more detailed explanation.

4. Move files into root

This change makes sense in this repository, where we have one file for everything: a CLI, a library itself and tests. Having bin, lib and test are unnecessary nesting.

5. Improve package.json

This change reorganizes fields in package.json to the "right" order, previously they were randomly placed. Also added a files field (https://docs.npmjs.com/files/package.json#files) to include only cli.js and index.js in npm package.

6. Add license

Previously license took a lot of space in the readme, making almost 90% of it. I extracted license to license file (not .md, since it's not markdown, but plain text), which brings useful things to our repository, such as:

screen shot 2017-07-26 at 3 42 02 pm

and

screen shot 2017-07-26 at 3 41 44 pm

7. Remove caching of dependencies on CI

It's always preferable for CI builds to be fresh, without any previous state. Sometimes this helps to catch some nasty bugs related to dependencies. By the way, location of dependencies to cache specified currently ~/.yarn-cache is out-dated, so Circle wasn't caching any Yarn dependencies.

@vadimdemedes vadimdemedes changed the title Improvements Improvements to the project Jul 26, 2017
@stephenmathieson
Copy link
Contributor

  1. 👍
  2. can we add the old release notes to the GH releases page? if not, it may make figuring out what happened between two versions more difficult than necessary
  3. iirc the folks at Yarn suggest leaving the lockfile there, but the explanation you provided sounds good to me
  4. 💯
  5. 👍
  6. 👍
  7. 👍

@vadimdemedes
Copy link
Author

vadimdemedes commented Jul 26, 2017

can we add the old release notes to the GH releases page? if not, it may make figuring out what happened between two versions more difficult than necessary

It would involve going through each tag and editing its release, which would be quite consuming. But from what I see, it's just a copy of the commit log, so users can still see what commits occurred between releases (tags).

iirc the folks at Yarn suggest leaving the lockfile there, but the explanation you provided sounds good to me

Yes, but this advice really works only for apps, not dependencies (libraries). They should clarify it in the docs.

@stephenmathieson Thanks for reviewing it so fast, I can now move on to the next things, which this PR was blocking :)

@vadimdemedes vadimdemedes merged commit 916cfe2 into master Jul 26, 2017
@vadimdemedes vadimdemedes deleted the improvements branch July 26, 2017 14:06
@stephenmathieson
Copy link
Contributor

related article: https://yarnpkg.com/blog/2016/11/24/lockfiles-for-all/ (seems to have been edited a bit since...)

and happy to review! you caught me at the perfect time today, haha

I'm excited about all of the work you're doing here. this is great stuff dude!

@vadimdemedes
Copy link
Author

Thanks @stephenmathieson, I'm enjoying it too :)

@f2prateek
Copy link
Contributor

Just noticed this:

  1. Remove History.md in favor of GitHub releases
    As discussed with @stevenmiller888, GitHub releases are a more pleasant way to write informative releases, unlike the History.md, which is basically a copy of commit log. Example of what you can do with releases: https://github.com/avajs/ava/releases.

I would still advocate for having a changelog file.

It might seem that the changelog in this repo was not particularly valuable, and that should have not happened, but that could easily happen with releases too (if all I did was copy the commit log). We use changelogs in other repos https://github.com/segmentio/analytics-android/blob/master/CHANGELOG.md#changelog which are not simply the commit logs.

In some repos we have maintained a "running" changelog of with commits. This made it easier for the person making the release, who didn't have to figure out what changed since the last release.

While Github releases are definitely nice, the biggest pain point is they're not automated. The latest release https://github.com/segmentio/analytics-node/releases/tag/v3.0.0 is already without a changelog.

Lastly I think consumers do expect to see a changelog/history file in the repo. We used to do releases for analytics-ios and people pointed it out that a changelog was missing.

@vadimdemedes
Copy link
Author

Sure, will add it back!

Personally I like writing release notes in releases, because it looks nicer: https://github.com/avajs/ava/releases.

The latest release v3.0.0 (release) is already without a changelog.

I was planning to write release notes today, because I was going off when the release came out.

@f2prateek
Copy link
Contributor

👍 I think there's little harm in adding both! For analytics-android I keep the changelog in https://github.com/segmentio/analytics-android/blob/master/CHANGELOG.md (which is not just a commit log), and then at the time of release just copy paste it into the releases tab https://github.com/segmentio/analytics-android/releases.

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.

3 participants