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 #4698

Closed
MylesBorins opened this issue Jan 14, 2016 · 15 comments
Closed

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

MylesBorins opened this issue Jan 14, 2016 · 15 comments
Labels
lts Issues and PRs related to Long Term Support releases.

Comments

@MylesBorins
Copy link
Contributor

I just did a run of branch diff and will put the results in a comment below. It appears there are 68 patch level commits that have landed on master that are not currently under LTS watch

branch-diff v4.x-staging master --exclude-label semver-major,semver-minor,lts-watch-v4.x,dont-land-on-v4.x --filter-release

We should likely audit these commits to see if we want to add them. I also think it would be a good idea for us to implement a new label to filter these results out in future audits. dont-land-on-v4.x?

@MylesBorins
Copy link
Contributor Author

@MylesBorins
Copy link
Contributor Author

/cc @jasnell @rvagg @Fishrock123

@MylesBorins MylesBorins added lts Issues and PRs related to Long Term Support releases. v4.x labels Jan 14, 2016
@rvagg
Copy link
Member

rvagg commented Jan 15, 2016

#2778 is an interesting case, it has landed-on-v4.x on it cause it was merged then I believe backed out. Is it time for a dont-land-on-v4.x flag to weed some of these out for good?

I think also it might be best to compare to v5.x rather than master, if it wasn't good enough for v5.x then it's not going to be good enough for v4.x.

@rvagg
Copy link
Member

rvagg commented Jan 15, 2016

@iarna @Fishrock123 and @nodejs/v8: I've made a new dont-land-on-v4.x tag and am applying it to npm@3 and [email protected] PRs so we can filter them out.

@rvagg
Copy link
Member

rvagg commented Jan 15, 2016

@iarna @Fishrock123 and @nodejs/v8: sorry, I meant to ask if you could please try and remember to tag new PRs with this label when we know they shouldn't touch v4.x.

@MylesBorins
Copy link
Contributor Author

I've updated the list above filtering for dont-land-on-v4.x as well

@rvagg
Copy link
Member

rvagg commented Jan 15, 2016

OK, I've done a heap of tagging and a bit of merging, if you grab the latest v5.x and run branch-diff against that you'll see the remaining list, which I have comments and questions on here (I think this covers it):

@jasnell
Copy link
Member

jasnell commented Jan 15, 2016

After having been delayed working on this by dealing with several fires this week, I'm hoping to be able to return attention to this later on today. @thealphanerd and @rvagg ... thank you both very much for continuing to press forward on it. I'm resetting my goal to have the 4.2.5 release proposal ready to go by end of the day Monday.

@MylesBorins
Copy link
Contributor Author

@rvagg thanks for going through this!

Regarding linting changes, we have been backporting them for now. I imagine the number of rules being added are going to be minimal. The churn though will not. Keeping up with these changes will make future backporting substantially easier

@targos
Copy link
Member

targos commented Jan 16, 2016

I cherry-picked the eslint update commits and fixed the merge conflicts: #4721

@Trott
Copy link
Member

Trott commented Jan 17, 2016

  • [6d6bc5d9d8] - test: mark http-pipeline-flood flaky (Rich Trott) #3616 — we should try and have a similar test quality on LTS as we do on master, I find it difficult to keep track of what should be flaky and what shouldn't. Perhaps @Trott can help with cherry-picking?

This is a one-line change that was subsequently undone by d9734b7 (which fixed the test so it wasn't flaky anymore). I'd recommend cherry-picking them both (so that everything lands cleanly) and landing.

In fact, here's a PR that does exactly that: #4730

UPDATE: Er, actually, these are superseded by a third commit that's already in v4.x-staging. Cherry-picking both of these commits results in no change. So, no need to land them, I guess.

@Trott
Copy link
Member

Trott commented Jan 17, 2016

This one cherry-picks cleanly to v4.x-staging. Here's a PR for good measure: #4732

@Trott
Copy link
Member

Trott commented Jan 17, 2016

Yup. If it's at all helpful, PR: #4734

I think that's the end of the stuff on the list that I was involved with. If there's something else I should look at, let me know.

@Trott
Copy link
Member

Trott commented Feb 11, 2016

@thealphanerd Is this issue still an active concern?

@MylesBorins
Copy link
Contributor Author

Closing in lieu of a new thread to be opened soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lts Issues and PRs related to Long Term Support releases.
Projects
None yet
Development

No branches or pull requests

5 participants