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

CLI - Remove deprecated use of the --project-path flag #915

Merged
merged 9 commits into from
Dec 30, 2024

Conversation

bfops
Copy link
Collaborator

@bfops bfops commented Mar 1, 2024

Description of Changes

Remove the deprecated use of --project-path to provide a wasm file.

API and ABI breaking changes

Yes, this breaks the CLI API.

Expected complexity level and risk

1

@bfops bfops changed the base branch from master to bfops/publish-wasm March 1, 2024 22:24
@bfops bfops mentioned this pull request Mar 1, 2024
7 tasks
@bfops bfops added the Do not merge Do not merge PRs with this label without coordinating further label Mar 14, 2024
@bfops
Copy link
Collaborator Author

bfops commented Mar 14, 2024

(do not merge until we're ready to make this breaking change!)

@bfops bfops removed the Do not merge Do not merge PRs with this label without coordinating further label May 24, 2024
@bfops bfops requested a review from jdetter May 24, 2024 15:14
@bfops bfops added the CLI only This change only affects the CLI behavior label May 24, 2024
Base automatically changed from bfops/publish-wasm to master May 24, 2024 15:15
@bfops bfops removed the request for review from jdetter May 24, 2024 15:17
@bfops bfops changed the base branch from master to mamcx/system_tables_newtype June 11, 2024 20:38
@bfops bfops changed the base branch from mamcx/system_tables_newtype to master June 11, 2024 20:39
@bfops bfops added Do not merge Do not merge PRs with this label without coordinating further and removed Do not merge Do not merge PRs with this label without coordinating further labels Jun 11, 2024
@bfops bfops changed the title Remove deprecated --project-path flag CLI - Remove deprecated use of the --project-path flag Sep 5, 2024
@bfops bfops added the api-break label Nov 8, 2024
@bfops bfops marked this pull request as ready for review November 8, 2024 21:47
@bfops bfops requested a review from cloutiertyler as a code owner November 8, 2024 21:47
Copy link
Contributor

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

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

Seems fine. Do we no longer allow you to specify the project for the wasm and just require you point directly to the wasm binary?

@bfops
Copy link
Collaborator Author

bfops commented Nov 9, 2024

Seems fine. Do we no longer allow you to specify the project for the wasm and just require you point directly to the wasm binary?

I'm not 100% sure what you're asking so I hope this is clear: we never allowed users to specify both. Previously, --project-path could take either a directory or a wasm path. Then we added --wasm-file (now --bin-path) to make the behavior clearer, but we still quietly supported taking a wasm file via --project-path. After this PR, you use --project-path to provide a directory, and --bin-path to provide a wasm file (and you still can't provide both at once).

@bfops bfops removed the api-break label Nov 11, 2024
kazimuth added a commit that referenced this pull request Nov 15, 2024
Fix CLI jankily

Remove dead comment

Implement name generation policy change (private #915 WIP)

Update bindings and cli to handle new name format

Clippy my nemesis

Finish updating csharp module bindings

And fix names

WIP

Placate csharpier

Final fixes

Fix schema doctests
kazimuth added a commit that referenced this pull request Nov 15, 2024
Fix CLI jankily

Remove dead comment

Implement name generation policy change (private #915 WIP)

Update bindings and cli to handle new name format

Clippy my nemesis

Finish updating csharp module bindings

And fix names

WIP

Placate csharpier

Final fixes

Fix schema doctests
kazimuth added a commit that referenced this pull request Nov 18, 2024
Fix CLI jankily

Remove dead comment

Implement name generation policy change (private #915 WIP)

Update bindings and cli to handle new name format

Clippy my nemesis

Finish updating csharp module bindings

And fix names

WIP

Placate csharpier

Final fixes

Fix schema doctests
kazimuth added a commit that referenced this pull request Nov 18, 2024
Fix CLI jankily

Remove dead comment

Implement name generation policy change (private #915 WIP)

Update bindings and cli to handle new name format

Clippy my nemesis

Finish updating csharp module bindings

And fix names

WIP

Placate csharpier

Final fixes

Fix schema doctests

Ah, C# is seeing the autogenerated names despite being on v8 still

Add test

WIP

Previous c# fix could not work due to v8. This one can work until the upgrade

Format
kazimuth added a commit that referenced this pull request Nov 20, 2024
Fix CLI jankily

Remove dead comment

Implement name generation policy change (private #915 WIP)

Update bindings and cli to handle new name format

Clippy my nemesis

Finish updating csharp module bindings

And fix names

WIP

Placate csharpier

Final fixes

Fix schema doctests

Ah, C# is seeing the autogenerated names despite being on v8 still

Add test

WIP

Previous c# fix could not work due to v8. This one can work until the upgrade

Format
kazimuth added a commit that referenced this pull request Nov 20, 2024
Fix CLI jankily

Remove dead comment

Implement name generation policy change (private #915 WIP)

Update bindings and cli to handle new name format

Clippy my nemesis

Finish updating csharp module bindings

And fix names

WIP

Placate csharpier

Final fixes

Fix schema doctests

Ah, C# is seeing the autogenerated names despite being on v8 still

Add test

WIP

Previous c# fix could not work due to v8. This one can work until the upgrade

Format
@bfops bfops added the api-break A PR that makes an API breaking change label Dec 2, 2024
@gefjon gefjon added this pull request to the merge queue Dec 30, 2024
Merged via the queue into master with commit 43ad8be Dec 30, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-break A PR that makes an API breaking change CLI only This change only affects the CLI behavior release-1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants