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

Maintain Script Update #29

Merged
merged 3 commits into from
Jun 22, 2023
Merged

Maintain Script Update #29

merged 3 commits into from
Jun 22, 2023

Conversation

momomomo111
Copy link
Contributor

I have made the following major changes:
• Modified to declare environment variables such as TOKEN and SHA from the outside
• Added the ability to assign someone else if the flag owner is not a collaborator in the Repository
• Removed unnecessary environment variables

スクリプトの変更
@momomomo111 momomomo111 self-assigned this Jun 22, 2023
).maintain(
locationSarif = "./lint-results.sarif",
labelName = "featureflag-expiration",
assignerAlternative = "{Alternative assignor's GitHub UserId}"
Copy link
Contributor

Choose a reason for hiding this comment

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

ちょっと長いですが、fallbackAssigneeWhenOwnerNotPresentにしちゃってもいいかも (現状コード見ないと何するのかよくわからない感じがあります。)

Copy link
Contributor

@takahirom takahirom Jun 22, 2023

Choose a reason for hiding this comment

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

Oh, I forgot to use English. But how about using fallbackAssigneeWhenOwnerNotPresent so that we can see what the code will do without seeing the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with that opinion. I know it's a little long winded, but I think it's a good idea to prioritize readability.

repoName = System.getenv("GITHUB_REPOSITORY"),
headSha = System.getenv("HEAD_SHA")
).maintain(
locationSarif = "./lint-results.sarif",
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using sarifLocation 👀

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry how about sarifFilePath

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got it.

Copy link
Contributor

@takahirom takahirom left a comment

Choose a reason for hiding this comment

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

Great

@momomomo111 momomomo111 merged commit 37c45e2 into main Jun 22, 2023
@momomomo111 momomomo111 deleted the update-maintain-script branch June 22, 2023 04:25
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.

2 participants