Skip to content

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

Merged
ndeloof merged 1 commit intocompose-spec:mainfrom
idsulik:fix-issue-12001
Jul 23, 2024
Merged

Revert "run interpolation after merge, but for required attributes"#666
ndeloof merged 1 commit intocompose-spec:mainfrom
idsulik:fix-issue-12001

Conversation

@idsulik
Copy link
Copy Markdown
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
@idsulik
Revert "run interpolation after merge, but for required attributes"
ff7d591 
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
@ndeloof
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

ndeloof commented Jul 23, 2024

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

@idsulik
Copy link
Copy Markdown
Contributor Author

idsulik commented Jul 23, 2024

Great!

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

jhrotko commented Jul 23, 2024

there are more issues than in #667

@ndeloof
Copy link
Copy Markdown
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
@Vigilans Vigilans mentioned this pull request Jan 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ndeloof ndeloof ndeloof approved these changes

@jhrotko jhrotko jhrotko approved these changes

@glours glours Awaiting requested review from glours

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@idsulik @ndeloof @jhrotko