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

re2c: 1.0.3 -> 1.2.1 #70946

Merged
merged 2 commits into from Oct 12, 2019
Merged

re2c: 1.0.3 -> 1.2.1 #70946

merged 2 commits into from Oct 12, 2019

Conversation

trofi
Copy link
Contributor

@trofi trofi commented Oct 10, 2019

No description provided.

Copy link
Member

@thoughtpolice thoughtpolice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Just one minor nit: please rebase this on top of the staging branch instead of master. This will introduce a delay (few days to ~2 weeks, IME) before you'll see 1.2.1 in master, but will save people a lot of grief by avoiding a lot of rebuilds. That's because re2c has some pretty deep transitive dependencies across the source tree, including stdenv.

A good rule of thumb (for me) is that if you're looking at ~1000 rebuilds, you might want to ask someone -- a lot of the decision can depend on the changes in question. But at ~2000 rebuilds or more in total, it should definitely go to staging. Our bot @GrahamcOfBorg will add some tags indicating how many rebuilds it estimates will happen, so you can get an idea of where it should go.

This change otherwise looks just fine -- thanks! (I'm only asking for 'request changes' to ensure someone doesn't accidentally hit the merge button before this is rebased.)

@thoughtpolice thoughtpolice self-assigned this Oct 11, 2019
@trofi trofi changed the base branch from master to staging October 11, 2019 19:10
@trofi
Copy link
Contributor Author

trofi commented Oct 11, 2019

The large rebuild impact makes sense. Rebased on to of staging.

Parallel builds speed up building considerably.
Tests don't add much of an overhead.

Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
@trofi
Copy link
Contributor Author

trofi commented Oct 11, 2019

While at it enabled parallel builds and tests.

@thoughtpolice thoughtpolice merged commit 697429c into NixOS:staging Oct 12, 2019
@c0bw3b c0bw3b mentioned this pull request Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants