-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[move-examples] Standardize move example dependencies to local #18009
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
base: main
Are you sure you want to change the base?
[move-examples] Standardize move example dependencies to local #18009
Conversation
25ac1f8 to
c6b6681
Compare
|
|
||
| [dependencies] | ||
| AptosFramework = { git = "https://github.com/aptos-labs/aptos-framework.git", subdir = "aptos-framework", rev = "mainnet" } | ||
| AptosFramework = { local = "../../framework/aptos-framework" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incorrect Relative Path to AptosFramework
Incorrect relative path to AptosFramework. The file is located at aptos-move/move-examples/resource_groups/secondary/Move.toml (3 levels deep), but uses ../../framework/aptos-framework which would resolve to aptos-move/move-examples/framework/aptos-framework. The correct path should be ../../../framework/aptos-framework to reach aptos-move/framework/aptos-framework, consistent with other examples at the same directory depth (e.g., token_objects/ambassador, dao/nft_dao, mint_nft subdirectories).
|
|
||
| [dependencies] | ||
| AptosFramework = { git = "https://github.com/aptos-labs/aptos-framework.git", subdir = "aptos-framework", rev = "mainnet" } | ||
| AptosFramework = { local = "../../framework/aptos-framework" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unfortunately also deadly, and we changed it from this to what we currently have a while ago.
Reason is users copy those examples for playing around and then link to some arbitrary local framework.
In fact normally it is NOT a problem to refer to main because of upgrade compatibility. Only in the very particular case of the option hack we have source level incompatible change, I doubt we ever have this again,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from option, framework removes the use of "::" because compiler will warn about it since 2.2. Unfortunately, the modified code cannot pass the compiler with a lower language version such as 2.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, on the other hand, this was also an exception. Normally we would never make a breaking change to the source language, this one time we allowed since hardly any user would have known the weird :: notation. Perhaps we should have also not even done this.
I'd go so far and argue our examples linking to ancient commits in the framework history, is actually a good test that a similar user project doesn't stop working in the future. We should embrace that this should always work, so the CLI is 100% downwards compatible.
Description
CI was getting stuck because of a mismatch in versions, this should instead run against the code here, debatably it would find some differences if it used two different ones, but would be best to leave that to some sort of automation to replace the dependencies.
How Has This Been Tested?
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist
Note
Switches Move example manifests to reference local Aptos framework/token libraries instead of git revisions, with minor manifest cleanups.
AptosFramework,AptosStdlib,AptosToken, andAptosTokenObjectsacross exampleMove.tomlfiles (e.g.,aggregator_examples,argument_example,bcs-stream,event,mint_nft/*,token_objects/*, etc.).[dependencies]blocks and update paths like../../framework/aptos-framework(and related framework crates).raffle,3-Adding-Admin,4-Getting-Production-Ready).Written by Cursor Bugbot for commit c6b6681. This will update automatically on new commits. Configure here.