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

Revert "run interpolation after merge, but for required attributes" #666

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

idsulik
Copy link
Contributor

@idsulik idsulik commented Jul 23, 2024

This reverts commit 65600ce.

The commit led to this issue docker/compose#12001, a quick fix is to revert the commit and release it.

@idsulik idsulik requested a review from ndeloof as a code owner July 23, 2024 08:54
@ndeloof
Copy link
Collaborator

ndeloof commented Jul 23, 2024

We only do such revert if there's no reasonable fix to offer.
For those impacted it is easy to just keep running older docker compose release

@idsulik
Copy link
Contributor Author

idsulik commented Jul 23, 2024

We only do such revert if there's no reasonable fix to offer. For those impacted it is easy to just keep running older docker compose release

Okay, I got it. It was just a quick fix. I think it'll be better to revert and release if we don't be able to fix it asap, because more and more users download the latest version and a lot of them will be forced to downgrade or spent time figuring out what went wrong

@ndeloof
Copy link
Collaborator

ndeloof commented Jul 23, 2024

Sure. A fix is proposed by @jhrotko here : #667

@idsulik
Copy link
Contributor Author

idsulik commented Jul 23, 2024

Great!

@idsulik idsulik closed this Jul 23, 2024
@jhrotko
Copy link
Collaborator

jhrotko commented Jul 23, 2024

there are more issues than in #667

@ndeloof
Copy link
Collaborator

ndeloof commented Jul 23, 2024

indeed. I've been investigating docker/compose#12001 and can't find a simple fix.
I suggest we revert this and introduce test cases to cover the identified issues, so that next attempt (would be 3rd one 😓) to implement "interpolate after merge" would be safer.

@ndeloof ndeloof requested review from glours and jhrotko July 23, 2024 12:29
@idsulik idsulik reopened this Jul 23, 2024
@ndeloof ndeloof merged commit bfaf10b into compose-spec:main Jul 23, 2024
15 checks passed
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

Successfully merging this pull request may close these issues.

3 participants