Skip to content

Add rules_shell fixing#1303

Merged
vladmos merged 2 commits intobazelbuild:mainfrom
keith:ks/add-rules_shell-fixing
Jan 17, 2025
Merged

Add rules_shell fixing#1303
vladmos merged 2 commits intobazelbuild:mainfrom
keith:ks/add-rules_shell-fixing

Conversation

@keith
Copy link
Member

@keith keith commented Oct 17, 2024

Currently these are split into 3 rules because 1 rule cannot add 3 load
statements and this repo doesn't have a defs.bzl that exposes them all

@keith
Copy link
Member Author

keith commented Oct 17, 2024

i don't feel strongly about the defs.bzl part since realistically these rules shouldn't be disabled, but happy to try to go that route and add that upstream to unify this into 1 rule

@keith keith force-pushed the ks/add-rules_shell-fixing branch from 9089c4e to 5445e37 Compare December 6, 2024 01:21
@keith keith force-pushed the ks/add-rules_shell-fixing branch from 5445e37 to 3c92714 Compare January 10, 2025 20:34
@keith
Copy link
Member Author

keith commented Jan 10, 2025

@vladmos @comius another one for you, CI issue is infra

Copy link
Contributor

@comius comius 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 this change!

if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return nil
}
return NotLoadedFunctionUsageCheck(f, fileReader, []string{"sh_binary"}, "@rules_shell//shell:sh_binary.bzl")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move hard coded "@rules_shell//shell:" into tables. See other warnings.

This makes it a bit easier to import, because we're overriding those labels internally.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah nice, i skipped that not knowing you were replacing that internally, i should have known! Updated!

@comius
Copy link
Contributor

comius commented Jan 13, 2025

cc @c-mita

Currently these are split into 3 rules because 1 rule cannot add 3 load
statements and this repo doesn't have a defs.bzl that exposes them all
@keith keith force-pushed the ks/add-rules_shell-fixing branch from 3c92714 to ccd4bc9 Compare January 13, 2025 18:10
@keith keith requested a review from comius January 13, 2025 18:11
@vladmos vladmos merged commit 3648bec into bazelbuild:main Jan 17, 2025
2 checks passed
@keith keith deleted the ks/add-rules_shell-fixing branch January 17, 2025 17:53
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.

3 participants