-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,5 +7,5 @@ resource_groups_primary = "_" | |
| resource_groups_secondary = "_" | ||
|
|
||
| [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 commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Incorrect Relative Path to AptosFrameworkIncorrect relative path to AptosFramework. The file is located at |
||
| ResourceGroupsPrimary = { local = "../primary" } | ||
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
optionhack 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.