Skip to content

chore: Rename Component::name_in_manifest and short_name_in_manifest method to name and short_name#4778

Merged
rami3l merged 2 commits intorust-lang:mainfrom
Cloud0310:rename-method
Mar 28, 2026
Merged

chore: Rename Component::name_in_manifest and short_name_in_manifest method to name and short_name#4778
rami3l merged 2 commits intorust-lang:mainfrom
Cloud0310:rename-method

Conversation

@Cloud0310
Copy link
Copy Markdown
Contributor

@Cloud0310 Cloud0310 commented Mar 26, 2026

These function on Component is not related to Manifest, so renaming them to distinguish against Manifest::name() and Manifest::short_name().

Founded this problem when I was dealing with #4768. By naming, two functions looks like should be the same, but they're actually different things.

ref: src/dist/manifest.rs#L55-L62 src/dist/manifest.rs#L55-L62

@Cloud0310 Cloud0310 changed the title refactor: Rename Component::name_in_manifest and short_name_in_manifest method to name and short_name chore: Rename Component::name_in_manifest and short_name_in_manifest method to name and short_name Mar 26, 2026
Copy link
Copy Markdown
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

Makes sense to me, thanks!

View changes since this review

Copy link
Copy Markdown
Member

@rami3l rami3l left a comment

Choose a reason for hiding this comment

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

@Cloud0310
Copy link
Copy Markdown
Contributor Author

Added new comments for clarify the usage of the functions in latest commit here.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

…t_name` accordingly

In `Component`, these methods are not related to the final name
associated with manifest.
On the `Manifest` side, it have `name` and `short_name` for get the
re-mapped component names.
So, I think renaming these func names in Component helps clarify the the
code.
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 28, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Cloud0310 Cloud0310 force-pushed the rename-method branch 2 times, most recently from b1a9a64 to d9c345d Compare March 28, 2026 13:13
Copy link
Copy Markdown
Member

@rami3l rami3l left a comment

Choose a reason for hiding this comment

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

LGTM, let's merge it :)

View changes since this review

@rami3l rami3l added this pull request to the merge queue Mar 28, 2026
@rami3l rami3l removed this pull request from the merge queue due to a manual request Mar 28, 2026
`Manifest::name` and the `short_name` funcc accordingly.
@rami3l rami3l enabled auto-merge March 28, 2026 19:18
@rami3l rami3l added this pull request to the merge queue Mar 28, 2026
Merged via the queue into rust-lang:main with commit 682bad2 Mar 28, 2026
29 checks passed
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.

4 participants