Skip to content

Instantly share code, notes, and snippets.

@sokra
Created January 31, 2017 19:25
Show Gist options
  • Star 35 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save sokra/bc283d7f931c9f59f33e18dd35c851f4 to your computer and use it in GitHub Desktop.
Save sokra/bc283d7f931c9f59f33e18dd35c851f4 to your computer and use it in GitHub Desktop.

My opinions

as open source maintainer:

Merge PR

Merge

I like normal "Merge" the most.

  • It keeps git history of the author.
  • It works great when multiple authors work on a PR.
  • git blame shows a smaller commit. More likely to understand what's the change intent.
  • git bisect finds bugs easier.
  • Easier to remove a part of the PR.

Author should feel free to squash logical related commits in her/his PR.

Squash to merge

  • You lose information.
  • Difficult to fix a PR based this PR or a intermediate state of it.

Rebase to merge

  • You rewrite commits.
  • Difficult to fix a PR based this PR or a intermediate state of it.

Commit messages

I don't like to have commit messages in a specific format.

  • It should be easy to write the message.
  • A PR author probably don't know how about your format.
    • Asking her/him to rewrite the commit message could be difficult. Not everybody is a git expert.
  • Commit message can't be wrong, because it's not relevant for the process
  • You don't need an extra tool to make a commit
  • Relevant information can be put into the PR. It's easily editable and can have a template. It also includes discussion.
  • Minimium work required to write and merge PRs.

Review

I like github reviews.

I don't think an extra tooling is needed here.

Release

I don't like to use a separate tool for releasing.

I like this command: npm version patch && git push && git push --tags && npm publish

  • It's simple. No setup needed. Works on every repo.
  • It creates tags in git and npm.
  • It fails if your local branch is not up-to-date.
  • It fails if you have uncommited changes.
  • You can manually fix-up every part of the process. Custom version, npm tag, etc.
  • You have to check CI manually before cutting a release. But probably you already have a notification in place if CI on master fails.

Nightly, automatic publish to npm

I don't like automatic publish to npm.

I think it's not nessecary (at least for npm packages).

Just use webpack/webpack#master in your package.json dependencies to get the bleeding edge version.

I don't like to leak my npm credentials to github or travis.

Automatic publish to gh-pages

I like to publish static sites automatically to gh-pages.

I think it's only relevant if you expect many commits.

Changelog

I like to write the change manually.

I don't like a CHANGELOG.md file.

I like github releases.

I like to write the changelog in one go.

  • You can omit unimportant messages
  • You can merge multiple commits (i. e. a PR fixing another PR)
  • You can add details to important stuff, i. e. steps for migration, links to documentation
  • You can edit the changelog if there was a typo in it
  • You don't have to rewrite git history to fix the changelog
  • Less context switching for interleaving changelog writing with coding. Only changelog work on release.
  • Fewest duplications
    • CHANGELOG.md would duplicate github releases
    • generating changelog from git history would duplicate git history in changelog
    • Details history: git history
    • Summarized changelog: github releases

Testing

I like integration tests over unit tests.

  • You get more for your time!
  • Refactoring is easier.
  • While refactoring it's less likely that you have to change the tests.
  • It's more likely to write correct integration tests than unit tests.
  • Integration tests can be used as examples for users.

Github projects

I think they are useless.

I like using labels and milestones instead.

  • Too many clicks
  • Duplicated information

Labels

I don't like to add many labels.

If issues have more the 3 labels I think that something is wrong. Remove labels!

In my opinion most useful labels:

  • bug it's a bug
  • merge it PR is fine, could be merged if CI passes
  • breaking change PR that as breaking changes
  • tests needed PR that need tests
  • Label for PRs another Contributor should take care of
  • Labels for issue where everybody can do a PR (easy one)

I like to assign important issues to the next milestone.

I like to assign important PRs to the next milestone.

Issues

I like to close issue if issue template is ignored.

I don't like trying to solve problems when information is incomplete.

I don't like to answer questions that many other people could answer. -> Stack Overflow

Transpiling

I don't like to write node.js code that need to be transpiled before use.

  • Extra step, extra source of errors
  • Slows down development
  • Debugging is more diffcult
  • Coverage is more difficult
  • People that want to contribute may not know your transpiled language
    • Don't assume everybody knows or likes Typescript or ES2021

It's fine if you need to bundle anyway, i. e. when writing code for the browser.

CI

I like CI.

  • Travis is great to linux test and general tests
  • Travis is slow for OSX build. Don't do more than 1 OSX build per commit.
    • Configure travis to only build master branch and PRs to reduce travis load.
  • Appveyor is great to Windows test. But it's also slow, because it only does one build at a time. Not as slow as travis for OSX.
    • Configure appveyor to only build master branch and PRs to reduce appveyor load.
  • CircleCI cannot replace travis, because it doesn't support matrix builds.
  • CircleCI can be used to fast feedback for PRs.

I think people don't read CI results for their PRs.

I like to copy relevant CI log seqment into the PR comments.

Coverage

I like code coverage.

  • codecov.io great, very detailed info
    • Can be configured to require 90% coverage on PR diffs
  • coveralls.io great, less detailed but fine
  • It breaks when doing too many builds in short time (you get no result)

Contributors

I don't like github teams.

I like contributors on the repo settings.

I like teams only if they affect multiple repos that are connected.

I like if only one person is responsible for release and publish.

I like to assign only one person for merging PRs. (not always possible)

  • Teams need too many click to configure for each repo
  • Similar structure of repos and teams is an antipattern
  • Teams can't be moved between orgs

Github configuration

I like to enable master as protected branch.

I like required status checks: travis, appveyor, coverage (patch > 90%).

Note: If it is also enforced for admins your can't push a new release commit

I like to disable "Squash to merge" and "Rebase to merge".

@wmertens
Copy link

wmertens commented Feb 1, 2017

I like this gist.

(But writing Node v4 compatible JS sucks. I transpile/bundle for the server as well, gets me a nice and fast bundle to run - faster start time.)

@anilanar
Copy link

anilanar commented Feb 1, 2017

In theory: If there are 4 units/modules with 3 edge cases in each, you would write 12 unit tests. If you write integration tests for those 4 units/modules, you must be aware of edge cases of all of them and potentially write 3^4=81 tests to cover edge cases. With integration tests, it's too easy to slip edge cases, where bugs usually occur. Integration tests usually test trivial functionality.

If you are refactoring in such a way that public interfaces of some units/modules change tremendously, obviously you need to rewrite unit tests.

@kentcdodds
Copy link

I like most of what you've got in here! I hope you don't mind me sharing my preferences/perspective too!

For context, I have 76 packages on npm (currently), most of which I'm actively maintaining, and I like to spend as little time maintining them as possible :)

Merge PR

I prefer Squash and merge because I feel like it keeps the git history clean and I've yet to wish that I had each commit individually. It's also nice because I can change the commit message when I merge (more on that next).

Commit messages

I like to standardize the format my commit messages. It allows for a great deal of automation (which saves me time in the long run). I use validate-commit-msg + husky + opt-cli. And if someone doesn't follow the format, that's fine because when I Squash and merge I can change the commit message anyway.

Release

I appreciate the simplicity of your release process. And your arguments are great. However, because saving time maintaining packages is paramount for me, I definitely automate my releases with semantic-release. It's been incredibly useful for me. There are many packages I have which I'm not actively developing (they're "done"), but I can continue to maintain them because when someone makes a PR, I can merge it (even on my phone) and it's automatically released. I often never think about releases. I really like not thinking about my version number too.

Changelog

I also don't like using CHANGELOG.md and prefer github releases. Because I use conventional-changelog commit message conventions, I can automatically generate this changelog and I think it looks pretty good. Not perfect, but again, because saving time is paramount, I think that this is an acceptable trade-off.

Testing

I agree that integration testing is better. I find it harder for me to get good code coverage with it. Also, it's more rewarding to write unit tests (even if they're less valuable). So I agree with you, and I should do it more, but I don't :-( Maybe I will!

Transpiling

I have a yeoman generator to initialize all my OSS projects. So the tooling setup is automatic, quick, and easy. And ever since I stopped committing generated files, I've never really had any trouble with contributions (note: I don't use TypeScript, just modern JavaScript). It's really nice to have all the latest stuff and not worry about Node/Browser version support and with the automatic tooling setup there's little reason to not.

Contributors

I've not had a ton of experience with GitHub teams. My angular-formly project is part of the formly-js org that I manage and teams haven't really been much of a problem. I thought I would mention in addition that all my projects use all-contributors. Not sure that would scale very well to a highly contributed project such as webpack (the community is huge), but it works great for my projects (and some have quite a few contributors).


Thanks for your thoughts! It's giving me some things to think about! I appreciate your perspective!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment