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

Node core auto labeling improvements #31

Closed
4 tasks done
phillipj opened this issue Apr 20, 2016 · 4 comments
Closed
4 tasks done

Node core auto labeling improvements #31

phillipj opened this issue Apr 20, 2016 · 4 comments

Comments

@phillipj
Copy link
Member

phillipj commented Apr 20, 2016

Low hanging fruits

  • doc/ for doc
  • benchmark/ for benchmark
  • deps/$name -> $name (libuv, v8, etc)
  • configure, *Makefile, node.gyp, common.gypi -> build (a lot of stuff in tools/ lives here too but not necessarily low hanging fruit)

To avoid duplicate work on these, please leave a comment if you'd like to give some of these a shot.

Challenges

Regarding the c++ label for any PR that has file changes in src/:

For example, there might be a PR that makes changes to both lib/net.js and src/tcp_wrap.cc, but the overall net change in lib/net.js is substantially greater than that of src/tcp_wrap.cc. In that particular case I would normally just tag it with net only.

How to work on this

Its current labels brain is located in lib/node-labels.js#resolveLabels(). It's even covered by tests test/node-labels.test.js.

Refs nodejs/node#6247

@Fishrock123
Copy link
Contributor

We may want to blacklist node_version.h from the c++ label if possible: nodejs/node#6383 (comment)

@jasnell
Copy link
Member

jasnell commented Apr 26, 2016

I've been considering adding a release label that we can use to tag release proposals. Would it be easier to key off that instead of blacklisting node_version.h?

@phillipj
Copy link
Member Author

@Fishrock123 @jasnell fixed it in #32 by ignoring src/node_version.h

@phillipj
Copy link
Member Author

Closing this for now as the originally mentioned labels has been fixed.

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

No branches or pull requests

3 participants