Skip to content
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

os: deprecate os.getwd in favor of os.get_current_dir (part 1) #22966

Merged
merged 7 commits into from
Nov 25, 2024

Conversation

Elsie19
Copy link
Contributor

@Elsie19 Elsie19 commented Nov 24, 2024

Fixes #21492.

Feel free to close this if you were thinking of something different or want to change it. I just made this PR to get something in the works.

Huly®: V_0.6-21407

vlib/os/os.c.v Outdated Show resolved Hide resolved
@Elsie19 Elsie19 requested a review from JalonSolov November 24, 2024 21:50
Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

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

Excellent work.

vlib/os/os_js.js.v Outdated Show resolved Hide resolved
@StunxFS
Copy link
Contributor

StunxFS commented Nov 24, 2024

I know what I'm about to say is out of the scope of this PR, but since this change is being made, why not do the same with chdir => set_current_dir?

@spytheman
Copy link
Member

I know what I'm about to say is out of the scope of this PR, but since this change is being made, why not do the same with chdir => set_current_directory?

No, os.chdir should stay as it is.

None of the reasons for getwd apply to chdir.

@Elsie19
Copy link
Contributor Author

Elsie19 commented Nov 24, 2024

I know what I'm about to say is out of the scope of this PR, but since this change is being made, why not do the same with chdir => set_current_directory?

Because the current name (getwd) is deprecated in C, and since these methods are taking the names of C functions, we have the chance to make this function name more platform agnostic.

@StunxFS
Copy link
Contributor

StunxFS commented Nov 24, 2024

Hm, I said that because if the user sees that os.get_current_dir exists, he might expect os.set_current_dir to exist as well (for consistency)

@Elsie19
Copy link
Contributor Author

Elsie19 commented Nov 24, 2024

chdir and getwd don't have anything in common name-wise, and neither would chdir and get_current_dir.

…d a few more commits are merged, to workaround a v-up-works-nix bug, and the `v -N -W build-tools` task in the CI
@spytheman spytheman changed the title os: deprecate getwd in favor of get_current_dir os: prepare for deprecating os.getwd in favor of os.get_current_dir Nov 25, 2024
@spytheman spytheman changed the title os: prepare for deprecating os.getwd in favor of os.get_current_dir os: deprecate os.getwd in favor of os.get_current_dir (part 1) Nov 25, 2024
@spytheman spytheman merged commit 9300982 into vlang:master Nov 25, 2024
76 checks passed
@medvednikov
Copy link
Member

V uses Go's stdlib as the golden standard:

https://pkg.go.dev/os#Getwd

It should stay os.getwd

spytheman added a commit that referenced this pull request Nov 25, 2024
@spytheman
Copy link
Member

Reverted in b801083.

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.

rename os.getwd -> os.getcwd
5 participants