Skip to content

Conversation

manolisliolios
Copy link
Contributor

@manolisliolios manolisliolios commented Sep 14, 2024

Description

Move things left and right (gets rid of the dot_move naming), and update our parsing logic to use our new naming standard.

The naming standard used is {ns_name}/{app_name}/{version} with /{version} being optional.

Test plan

Tweaked the existing e2e & unit tests to work with the new naming standard, and resolution works as expected on the e2e ones.


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:
  • REST API:

Copy link

vercel bot commented Sep 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 16, 2024 4:47pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 4:47pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 4:47pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 4:47pm

@manolisliolios manolisliolios self-assigned this Sep 16, 2024
Copy link
Contributor

@amnn amnn left a comment

Choose a reason for hiding this comment

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

Main comment is regarding how the parsing for VersionedName is done -- rest is nits. I'm also a little bit on the fence about using :v as the delimiter for versions, seeing it in practice, especially in the context of types, LMKWYT.

Copy link
Contributor

@amnn amnn left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turnaround @manolisliolios ! Given our offline chat, I'm still leaning towards /1 vs :v1 for the version identifiers, but will leave the final decision with you!

@manolisliolios manolisliolios force-pushed the ml/mvr-new-naming-standard branch from e3c679f to d4947ab Compare September 16, 2024 16:45
@manolisliolios manolisliolios merged commit 5771116 into main Sep 16, 2024
50 checks passed
@manolisliolios manolisliolios deleted the ml/mvr-new-naming-standard branch September 16, 2024 17:18
suiwombat pushed a commit that referenced this pull request Sep 16, 2024
## Description 

Move things left and right (gets rid of the `dot_move` naming), and
update our parsing logic to use our new naming standard.

The naming standard used is `{ns_name}/{app_name}/{version}` with
`/{version}` being optional.

## Test plan 

Tweaked the existing e2e & unit tests to work with the new naming
standard, and resolution works as expected on the e2e ones.

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
manolisliolios added a commit that referenced this pull request Sep 26, 2024
Move things left and right (gets rid of the `dot_move` naming), and
update our parsing logic to use our new naming standard.

The naming standard used is `{ns_name}/{app_name}/{version}` with
`/{version}` being optional.

Tweaked the existing e2e & unit tests to work with the new naming
standard, and resolution works as expected on the e2e ones.

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
- [ ] REST API:
manolisliolios added a commit that referenced this pull request Sep 26, 2024
Move things left and right (gets rid of the `dot_move` naming), and
update our parsing logic to use our new naming standard.

The naming standard used is `{ns_name}/{app_name}/{version}` with
`/{version}` being optional.

Tweaked the existing e2e & unit tests to work with the new naming
standard, and resolution works as expected on the e2e ones.

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
- [ ] REST API:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants