Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix sorting for 0.9.16 #4796

Closed
wants to merge 8 commits into from
Closed

Conversation

eskimor
Copy link
Member

@eskimor eskimor commented Jan 27, 2022

No description provided.

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Jan 27, 2022
@ordian
Copy link
Member

ordian commented Jan 27, 2022

could you backport the rest of the commits of #4795?

@chevdor
Copy link
Contributor

chevdor commented Jan 27, 2022

Please no auto merge here, I would like to update the substrate deps once paritytech/substrate#10742 is merged and then merge that one.

@eskimor
Copy link
Member Author

eskimor commented Jan 27, 2022

could you backport the rest of the commits of #4795?

I really don't want to. I checked, functionality wise it is the same. This waiting for CI to finally carry on with tasks is driving me crazy - I want that to finally stop.

@eskimor
Copy link
Member Author

eskimor commented Jan 27, 2022

I would prefer a workflow next time, where fixes just go to the release branch and once the release is done, we merge the release branch to master. - Would be way less tedious and error prone. I don't care if @drahnr disagrees 😉 - this manually keeping stuff in sync is just crazy. If we just open a PR, like any other, it would also be quite hard to forget that merge.

@drahnr
Copy link
Contributor

drahnr commented Jan 27, 2022

If we only have one release at a time, this might work sufficiently well, iff and only iff master doesn't move too fast. As soon as one has more than one release branch at the same time or master picking up with our team growth, this will become a mess.

@ordian
Copy link
Member

ordian commented Jan 27, 2022

could you backport the rest of the commits of #4795?

I really don't want to. I checked, functionality wise it is the same. This waiting for CI to finally carry on with tasks is driving me crazy - I want that to finally stop.

It seems the real problem here is long CI times. My main concern if we're to backport some urgent changes for 0.9.17 to 0.9.16 branch we might end up fixing some merge conflicts, which is unnecessary.

@eskimor
Copy link
Member Author

eskimor commented Jan 27, 2022

If we only have one release at a time, this might work sufficiently well, iff and only iff master doesn't move too fast. As soon as one has more than one release branch at the same time or master picking up with our team growth, this will become a mess.

Once the release is out, this is a different thing- then I am actually fine with back-porting. I am only concerned with the release branch prior to release.

@eskimor
Copy link
Member Author

eskimor commented Jan 27, 2022

It seems the real problem here is long CI times. My main concern if we're to backport some urgent changes for 0.9.17 to 0.9.16 branch we might end up fixing some merge conflicts, which is unnecessary.

Yes, long CI times are a big source of delays. Amplified by unnecessary/unknown changes, which cause PRs to fail, although they worked fine when based on master. So you end up not only waiting once for CI, but several times for a single stupid PR, that actually was already fine on master.

@chevdor chevdor added this to the v0.9.16 milestone Jan 28, 2022
chevdor and others added 3 commits January 28, 2022 10:18
* Add codeden-units=1

ref #4311

* opt-level to 3

* Fix opt-level

* Refactor apt-level into the release profile

fix #4780
@chevdor
Copy link
Contributor

chevdor commented Jan 28, 2022

Could you please change the base so we merge this PR into wk-prep-v0.9.16-rc7 ?

@eskimor eskimor closed this Jan 28, 2022
@chevdor chevdor mentioned this pull request Jan 28, 2022
@chevdor chevdor deleted the rk-fix-sorting-0.9.16 branch January 28, 2022 11:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants