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

Add CONTRIBUTING guide? #40

Closed
shelldandy opened this issue Nov 26, 2017 · 13 comments
Closed

Add CONTRIBUTING guide? #40

shelldandy opened this issue Nov 26, 2017 · 13 comments

Comments

@shelldandy
Copy link

I'm trying to do a small PR but i can't seem to figure out what npm run check-changelog wants me to do 😢

@ljharb
Copy link
Collaborator

ljharb commented Nov 26, 2017

https://github.com/airbnb/babel-plugin-dynamic-import-node/blob/master/package.json#L18 - it just checks to see that you've updated the changelog (in the Unreleased section)

@shelldandy
Copy link
Author

Yeah I figured that out and also figured you can't use npm version XXX but rather the custom commands, but I mean something as plain as adding in the readme:

* Write code
* `npm test`
* Adjust code

If you need to publish:
* npm run version:XXXXX
* npm publish

Something along those lines 😛

@shelldandy
Copy link
Author

Also semi-related I think it'd be nice be able to push some beta packages with the beta tag or something...

npm tag v.2.0.0-beta.1
npm publish --tag beta

So we could install babel-plugin-dynamic-import-node@beta and maybe try it with babel 7 to prepare and test our releases as well?

@shelldandy
Copy link
Author

In the meantime I added babel-plugin-dynamic-import-node-babel-7

On npm to try things out, once you publish v2 or beta version I can tweak the readme to point here back again 😄

@ljharb
Copy link
Collaborator

ljharb commented Nov 26, 2017

Either way tho, npm version is the command you run when publishing; PRs should never ever ever update the version number of the package.

Since babel 7 isn't out yet, we won't support it just yet - a PR to support it is premature.

@shelldandy
Copy link
Author

npm version v2.0.0-beta.1 doesn't work since you can't run that command with a dirty git status that the npm run check-changelog requires

@ljharb
Copy link
Collaborator

ljharb commented Nov 26, 2017

@Mike3run right but a) that command isn't that great, and versions should be bumped manually, and b) it's not supposed to be ran in this repo by anyone other than project maintainers :-)

Also, in general, it's considered polite to either ask before publishing a fork, and/or to publish it under a scope. It's not too late to unpublish yours ¯\_(ツ)_/¯

Separately, if this plugin doesn't work under babel 7, then filing a PR to fix that is probably much more productive than creating a fork.

@shelldandy
Copy link
Author

it's not supposed to be ran in this repo by anyone other than project maintainers :-)

If you suddenly lost interest in maintaining the project or any other reasons i think its still good idea to have the guide, for every other contributor in the future and even yourself.

About the fork

I made an issue to talk about babel 7 #39 but i was told it wasn't going to happen until babel 7 stable is out and got closed right away, so i wanted to give it a try anyways since it was still early sunday.

I added a bit of a redirect in the readme there to point back here if needed, and when this package is fully updated i'll just add it as a dependency and pretty much just do nothing other than handing it over to here.

@ljharb
Copy link
Collaborator

ljharb commented Nov 27, 2017

You can give things a try by npm installing from a github branch; there's no need to publish anything.

@shelldandy
Copy link
Author

Very true! I forgot about that

I guess it's a good moment to namespace your packages like babel/shopify/etc are doing

  • @airbnb/babel-plugin-dynamic-import-node
  • @airbnb/babel-preset

etc ?

@ljharb
Copy link
Collaborator

ljharb commented Nov 27, 2017

That doesn't really make sense to me when the package name doesn't really relate to the scope; by attaching the company name, it means we can't ever truly hand off the package to someone else if we no longer wish to maintain it.

@shelldandy
Copy link
Author

fair point

@shelldandy
Copy link
Author

🤷‍♂️

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

No branches or pull requests

2 participants