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

💥 api: align Container.withNewFile signature with Directory.withNewFile #7293

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

helderco
Copy link
Contributor

@helderco helderco commented May 6, 2024

Warning

This is a breaking change in Go and TypeScript1! 💥

Deprecation path not possible. May want to hold merge for v0.12.0.

Fixes #6961

Brings more consistency. We made contents mandatory in Directory.withNewFile but not in Container.withNewFile:

Footnotes

  1. May need to double check on TS. Not a breaking change for Python. ↩

@helderco helderco added the kind/breaking Breaking Change label May 6, 2024
@helderco helderco requested review from a team as code owners May 6, 2024 10:56
@gerhard gerhard added this to the v0.12.0 milestone May 6, 2024
@helderco helderco force-pushed the helder/oss-202-signature-misalignment-of-directorywith_new_file-and branch 4 times, most recently from 9bfb361 to 8fcad31 Compare May 7, 2024 13:06
@shykes
Copy link
Contributor

shykes commented May 24, 2024

The consistency argument is pretty compelling. If it was a good idea in Directory, we kind of have to do it in Container (I guess it was an oversight that we didn't to both at the same time).

@helderco helderco force-pushed the helder/oss-202-signature-misalignment-of-directorywith_new_file-and branch from 8fcad31 to f26353d Compare May 29, 2024 07:16
…ile`

Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
@helderco helderco force-pushed the helder/oss-202-signature-misalignment-of-directorywith_new_file-and branch from f26353d to bf3e9f3 Compare May 29, 2024 07:21
* @param opts.permissions Permission given to the written file (e.g., 0600).
* @param opts.owner A user:group to set for the file.
*
* The user and group can either be an ID (1000:1000) or a name (foo:bar).
*
* If the group is omitted, it defaults to the same as the user.
*/
withNewFile = (path: string, opts?: ContainerWithNewFileOpts): Container => {
Copy link
Member

Choose a reason for hiding this comment

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

Yes I can confirm this is a breaking change for TypeScript.
The parameter contents moved from a struct param into a require unary parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/breaking Breaking Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signature misalignment of Directory.with_new_file and Container.with_new_file
4 participants