Skip to content

Directly add extension instead of using Path::with_extension #125406

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented May 22, 2024

Path::with_extension has a nice footgun when the original path doesn't contain an extension: Anything after the last dot gets removed.

@rustbot
Copy link
Collaborator

rustbot commented May 22, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 22, 2024
@rust-log-analyzer

This comment has been minimized.

@tbu- tbu- force-pushed the pr_rm_path_with_extension branch from 70d39cd to 87b77a2 Compare May 22, 2024 13:30
@Nadrieril
Copy link
Member

Good spot!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 22, 2024

📌 Commit 87b77a2 has been approved by Nadrieril

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 22, 2024
fmease added a commit to fmease/rust that referenced this pull request May 22, 2024
…Nadrieril

Directly add extension instead of using `Path::with_extension`

`Path::with_extension` has a nice footgun when the original path doesn't contain an extension: Anything after the last dot gets removed.
fmease added a commit to fmease/rust that referenced this pull request May 22, 2024
…Nadrieril

Directly add extension instead of using `Path::with_extension`

`Path::with_extension` has a nice footgun when the original path doesn't contain an extension: Anything after the last dot gets removed.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#122665 (Add some tests for public-private dependencies.)
 - rust-lang#125210 (Cleanup: Fix up some diagnostics)
 - rust-lang#125316 (Tweak `Spacing` use)
 - rust-lang#125401 (Migrate `run-make/rustdoc-scrape-examples-macros` to `rmake.rs`)
 - rust-lang#125406 (Directly add extension instead of using `Path::with_extension`)
 - rust-lang#125409 (Rename `FrameworkOnlyWindows` to `RawDylibOnlyWindows`)
 - rust-lang#125416 (Use correct param-env in `MissingCopyImplementations`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 23, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#122665 (Add some tests for public-private dependencies.)
 - rust-lang#125210 (Cleanup: Fix up some diagnostics)
 - rust-lang#125316 (Tweak `Spacing` use)
 - rust-lang#125401 (Migrate `run-make/rustdoc-scrape-examples-macros` to `rmake.rs`)
 - rust-lang#125406 (Directly add extension instead of using `Path::with_extension`)
 - rust-lang#125409 (Rename `FrameworkOnlyWindows` to `RawDylibOnlyWindows`)
 - rust-lang#125416 (Use correct param-env in `MissingCopyImplementations`)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors rollup=iffy

@bors
Copy link
Collaborator

bors commented May 23, 2024

⌛ Testing commit 87b77a2 with merge bba736a...

bors added a commit to rust-lang-ci/rust that referenced this pull request May 23, 2024
…drieril

Directly add extension instead of using `Path::with_extension`

`Path::with_extension` has a nice footgun when the original path doesn't contain an extension: Anything after the last dot gets removed.
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented May 23, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 23, 2024
@tbu- tbu- force-pushed the pr_rm_path_with_extension branch from 87b77a2 to 0a3ecea Compare May 24, 2024 09:49
@tbu-
Copy link
Contributor Author

tbu- commented May 24, 2024

Anything after the last dot gets removed.

Interesting. This was apparently already hit by the existing code, because the added suffix just got dropped entirely. ^^

@Nadrieril
Copy link
Member

Do you think that might have been intentional?

@tbu-
Copy link
Contributor Author

tbu- commented May 24, 2024

Do you think that might have been intentional?

I don't think so. There's no point in adding a suffix that is immediately removed again. The name_suffix was purely dead code.

@Nadrieril
Copy link
Member

Alright, let's try again

@bors r+

@bors
Copy link
Collaborator

bors commented May 29, 2024

📌 Commit 0a3ecea has been approved by Nadrieril

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 29, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 29, 2024
@bors
Copy link
Collaborator

bors commented May 29, 2024

⌛ Testing commit 0a3ecea with merge f839e1d...

bors added a commit to rust-lang-ci/rust that referenced this pull request May 29, 2024
…drieril

Directly add extension instead of using `Path::with_extension`

`Path::with_extension` has a nice footgun when the original path doesn't contain an extension: Anything after the last dot gets removed.
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented May 29, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 29, 2024
@fmease
Copy link
Member

fmease commented May 30, 2024

sync @bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 30, 2024
@Nadrieril
Copy link
Member

The test failure is due to formatting, I'm surprised that tidy didn't catch it. Anyway, please run rustfmt and we should be good

`Path::with_extension` has a nice footgun when the original path doesn't
contain an extension: Anything after the last dot gets removed.
@tbu- tbu- force-pushed the pr_rm_path_with_extension branch from 0a3ecea to f7c51a2 Compare June 4, 2024 20:12
@tbu-
Copy link
Contributor Author

tbu- commented Jun 4, 2024

Autoformatted.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 4, 2024
@Nadrieril
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 5, 2024

📌 Commit f7c51a2 has been approved by Nadrieril

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2024
@bors
Copy link
Collaborator

bors commented Jun 6, 2024

⌛ Testing commit f7c51a2 with merge 67caf52...

@bors
Copy link
Collaborator

bors commented Jun 6, 2024

☀️ Test successful - checks-actions
Approved by: Nadrieril
Pushing 67caf52 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 6, 2024
@bors bors merged commit 67caf52 into rust-lang:master Jun 6, 2024
7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone Jun 6, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (67caf52): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 3.6%, secondary 4.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
3.6% [3.6%, 3.6%] 1
Regressions ❌
(secondary)
4.4% [4.4%, 4.4%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.6% [3.6%, 3.6%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 672.761s -> 670.655s (-0.31%)
Artifact size: 319.40 MiB -> 319.39 MiB (-0.00%)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants