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 major, minor, and patch labels to PRs, Issues #69

Closed
kenperkins opened this issue Dec 4, 2014 · 13 comments
Closed

Add major, minor, and patch labels to PRs, Issues #69

kenperkins opened this issue Dec 4, 2014 · 13 comments

Comments

@kenperkins
Copy link
Contributor

I think it would be nice to be able to label (and filter) PRs & issues, and to have some insight before even reviewing changes as to how they impact SemVer. There are a lot of issues and PRs that have already been submitted and there's little context to them on real or potential impact to SemVer.

I opened this in part to help to understand impact of an issue, say #66, which I assume to be major at such time as they're actually removed.

@indutny
Copy link
Member

indutny commented Dec 4, 2014

Good idea, +1

@brendanashworth
Copy link
Contributor

This would be a good addition and make it easier for a new contributor to help out. 👍

@nelsonpecora
Copy link
Contributor

👍 I think that would be helpful for maintainers, contributors, and users (who watch PRs that they want/need)

@nelsonpecora
Copy link
Contributor

I am not good at interacting with robots. Glad this is getting some traction, though!

@piscisaureus
Copy link
Contributor

+1 from me too.

As possible refinement, we could tag issues "bug fix", "api change" and "api addition". I think that's more clear for people who aren't thinking about semver all day long.

@kenperkins
Copy link
Contributor Author

@piscisaureus a risk of not being explicit with semver-matching labels is someone making a mistake and not realizing that there is an implicit meaning. For example, api-change doesn't always mean breaking.

@nelsonpecora
Copy link
Contributor

Yeah, I'm wary of using non-semver terminology. major means that there's an api change, and that api change is breaking. It's much more defined and explicit.

@rvagg
Copy link
Member

rvagg commented Jan 12, 2015

We should make this happen, it's going to be really hard to track which number we should bump for each release beyond 1.0.0, it would be nice to just search through PRs for these labels.

How about: semver-major, semver-minor, semver-patch and every PR must be labelled with one of these, just like the commits have to have the additional metadata in them. The person responsible for a release can then go and hound mergers who haven't labelled PRs between releases.

@nelsonpecora
Copy link
Contributor

👍 to semver-major, semver-minor, and semver-patch

@chrisdickinson
Copy link
Contributor

It would be sort of neat if we could automate the addition of labels by running the tests from the tip of iojs against the PR tip -- if they pass, we can safely label it minor or patch. Otherwise, it's a major bump.

For now I don't see any problem with collaborators adding these labels where appropriate, the above suggestion is strictly a "for the future" sort of thing :)

@brendanashworth
Copy link
Contributor

+1 on the general idea, but from what @chrisdickinson is saying, we'd have
to do it more reliably - a failed test could mean a major bump or just a
bad PR. However I think these tags would be really sweet for managing the
PRs.

A repository I think does a great job managing PRs and issues would be
TryGhost/Ghost, if anyone wants to take a look.

On Monday, January 12, 2015, Chris Dickinson [email protected]
wrote:

It would be sort of neat if we could automate the addition of labels by
running the tests from the tip of iojs against the PR tip -- if they pass,
we can safely label it minor or patch. Otherwise, it's a major bump.

For now I don't see any problem with collaborators adding these labels
where appropriate, the above suggestion is strictly a "for the future" sort
of thing :)


Reply to this email directly or view it on GitHub
#69 (comment).

@Fishrock123
Copy link
Contributor

We now have semver-minor and semver-major.

@cjihrig cjihrig closed this as completed Jan 23, 2015
@kenperkins
Copy link
Contributor Author

w00t! Thanks everyone.

kunalspathak added a commit to kunalspathak/node that referenced this issue Feb 26, 2017
Sync to nodejs/master (2016-05-05) at 330ea76

PR-URL: nodejs#69
Reviewed-By: Jianchun Xu <[email protected]>
kunalspathak added a commit to kunalspathak/node that referenced this issue Feb 26, 2017
Added APIs `GetTarget()`, `GetHandler()` for Proxy. Today, there is no
way to extract these fields through JSRT APIs, these APIs return
`undefined` for now. I have opened chakra-core/ChakraCore#950 to track
this.

Likewise, as per ES6 spec, there is no way to detect if an object is a Proxy
hence `IsProxy()` too needs an equivalent JSRT API.

PR-URL: nodejs#69
Reviewed-By: Jianchun Xu <[email protected]>
kunalspathak added a commit to kunalspathak/node that referenced this issue Feb 26, 2017
Fixed `test-process-versions` after merge.

PR-URL: nodejs#69
Reviewed-By: Jianchun Xu <[email protected]>
grahamkennery added a commit to grahamkennery/node that referenced this issue Nov 20, 2020
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

10 participants