-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
build: rename std.Build.*Step to std.Build.Step.* #15020
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
Conversation
const CheckFileStep = @This(); | ||
const std = @import("std"); | ||
const Step = std.Build.Step; | ||
const fs = std.fs; | ||
const mem = std.mem; |
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.
Saw const
at bottom or at the top of the files...
There are more const
at the top for Steps files so I took advantage by doing this PR to make things more consistent.
const std = @import("../std.zig"); | ||
const std = @import("std"); |
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.
Interesting. Should files in the std
module always be importing std by alias like this now? If so what's the reason(s)? I'm curious if non-std modules should be adopting the same convention.
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.
Just to note in order to do this type of thing in non-std modules you have to add the module to its own dependencies: #14708 (comment)
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.
I was wondering if we could avoid relative path on std import. It seems yes, we are already doing this in lib/std/Build/Cache.zig
Line 76 in 5df31f3
const std = @import("std"); |
@andrewrk do you want me to resolve conflicts before reviewing that PR? |
Follow-up actions from ziglang#14647 Fixes ziglang#14947
pub const CheckFileStep = @import("Build/Step/CheckFile.zig"); | ||
pub const CheckObjectStep = @import("Build/Step/CheckObject.zig"); | ||
pub const ConfigHeaderStep = @import("Build/Step/ConfigHeader.zig"); | ||
pub const FmtStep = @import("Build/Step/Fmt.zig"); | ||
pub const InstallArtifactStep = @import("Build/Step/InstallArtifact.zig"); | ||
pub const InstallDirStep = @import("Build/Step/InstallDir.zig"); | ||
pub const InstallFileStep = @import("Build/Step/InstallFile.zig"); | ||
pub const ObjCopyStep = @import("Build/Step/ObjCopy.zig"); | ||
pub const CompileStep = @import("Build/Step/Compile.zig"); | ||
pub const OptionsStep = @import("Build/Step/Options.zig"); | ||
pub const RemoveDirStep = @import("Build/Step/RemoveDir.zig"); | ||
pub const RunStep = @import("Build/Step/Run.zig"); | ||
pub const TranslateCStep = @import("Build/Step/TranslateC.zig"); | ||
pub const WriteFileStep = @import("Build/Step/WriteFile.zig"); |
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.
these decls are in the right file location but not the right namespace and does not implement the title as stated
For future reference when I request a review from myself, it is a polite way to say "please don't merge this until I approve it" |
Follow-up actions from #14647
Fixes #14947