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

Audit commits not found on v4.x-staging #7116

Closed
MylesBorins opened this issue Jun 2, 2016 · 4 comments
Closed

Audit commits not found on v4.x-staging #7116

MylesBorins opened this issue Jun 2, 2016 · 4 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@MylesBorins
Copy link
Contributor

MylesBorins commented Jun 2, 2016

branch-diff v4.x-staging master --exclude-label semver-major,semver-minor,lts-watch-v4.x,dont-land-on-v4.x --filter-release
  • [4fe1d6e75f] - unix,stream: fix getting the correct fd for a handle (Saúl Ibarra Corretgé) #6753
@MylesBorins
Copy link
Contributor Author

This list is about 1/3 of what there was yesterday when I started auditing... @nodejs/collaborators we need to be doing a better job with our LTS labelling

@MylesBorins MylesBorins added v4.x meta Issues and PRs related to the general management of the project. labels Jun 2, 2016
@Trott
Copy link
Member

Trott commented Jun 2, 2016

@thealphanerd We don't explain LTS labeling very much in our onboarding and I would not be surprised if most collaborators know little or nothing about it. There's a brief note in https://github.com/nodejs/node/blob/master/doc/onboarding-extras.md#labels. Perhaps you (or James or someone else who understands it better than most) can expand on that note and/or put a clear procedure in the main doc at https://github.com/nodejs/node/blob/master/doc/onboarding.md. Something clear and algorithmic would be awesome. If it requires squishy judgments and there's no way around it, then just noting that and moving on is probably fine.

Maybe put a rough draft in a comment on this issue? As clearly as you can state, what do you want collaborators to do in response to this issue and in the future? For example:

[f85412d] - doc: improvements to child_process, process docs (Alexander Makarenko)

What, if anything, should Alexander do about that at this time? And if it's not the same thing as what should be done in the first place, then what should he do differently in the future?

I definitely do not give any significant time to LTS labeling in the onboarding sessions I've done, so my apologies about that, and let's fix it.

@MylesBorins
Copy link
Contributor Author

@Trott I'll work on better docs. TLDR;

  • If you think something should be considered for LTS include lts-watch
  • If you think something is definitely not LTS include dont-land
  • If you are unsure ping @nodejs/lts or just leave it to be audited later

The above applies to everyone dealing with an incoming PR at any point in the review cycle

As for f85412d it looks like it is popping up as a false positive due to the lake of PR-URL. I am considering trying to introduce another fallback mechanism into our release tooling to allow us to add meta data that can be followed to the commits comment log.

@MylesBorins
Copy link
Contributor Author

Ok we are all done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

2 participants