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

Don't break Y1-Production with impactful changes to main #361

Closed
MTCam opened this issue May 24, 2021 · 4 comments
Closed

Don't break Y1-Production with impactful changes to main #361

MTCam opened this issue May 24, 2021 · 4 comments

Comments

@MTCam
Copy link
Member

MTCam commented May 24, 2021

Changes such as those proposed in #355 and #360 have profound impacts on downstream developments. Any such change proposals should include some attention to how those changes impact the year's production branch (y1-production this year).

In the interest of keeping prediction-critical technologies up-to-date with MIRGE-Com main, perhaps changes which significantly affect production-critical branches should come along with corresponding PRs against those branches as well.

@MTCam MTCam added this to To do in Y1-Production via automation May 24, 2021
@inducer
Copy link
Contributor

inducer commented May 24, 2021

I think the double-PR strategy is ultimately bound to lead to a merge nightmare. (assuming that what you mean is that the changes would be introduced redundantly, which is basically unavoidable given that this repo is configured to disallow merge commits... cc @matthiasdiener)

Personally, I think it would be reasonable to have a notion of where these go in the current "tower" of changes leading to the production branch, and keep them there only.

@thomasgibson
Copy link
Contributor

thomasgibson commented May 24, 2021

PRs like #354 #355 #360 are future-proofing mirgecom and I would argue necessary to keep up with further developments (e.g. lazy evaluation). I don't mind filing a PR that corrects the new experiments/operators in Y1.

@MTCam
Copy link
Member Author

MTCam commented May 24, 2021

PRs like #354 #355 #360 are future-proofing mirgecom and I would argue necessary to keep up with further developments (e.g. lazy evaluation). I don't mind filing a PR that corrects the new experiments/operators in Y1.

I'm certainly not intending that we don't DO those PRs - just that we need a sense of how production will be impacted before we merge stuff that requires sweeping changes. In general, my proposal is that we rebase a new production tower on the proposed new main as a part of the work required to prepare an impactful change for merge into main.

My intention is to go to PR interface, and click the "Y1" tag. There, all the branches that feed y1-production show up. Starting with the base, navier-stokes + av + disc, I can make the "new" navier-stokes that reflects the proposed changes in #354, then go to av and merge new ns, all the way up the chain to y1-production.

Doing this is not extra work, it is the required work to bring those branches up-to-date after main changes. The work we all do to keep our PRs up-to-date. My suggestion is that we do that work up front before the change to main is approved and pushed.

@thomasgibson
Copy link
Contributor

thomasgibson commented Jul 16, 2021

I think we can close this now that we have testing set up for y1-production (related PRs: #423, #437)

Y1-Production automation moved this from To do to Done Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants