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

Created welcome Message yml #520

Merged
merged 3 commits into from
Mar 19, 2023
Merged

Created welcome Message yml #520

merged 3 commits into from
Mar 19, 2023

Conversation

Alex-is-Gonzalez
Copy link
Collaborator

Created welcome message upon creating a first issue and first pr.

Created welcome message upon creating a first issue and first pr.
@Alex-is-Gonzalez Alex-is-Gonzalez requested a review from a team as a code owner March 2, 2023 22:48
Copy link
Member

@paramsiddharth paramsiddharth left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@JamesMGreene Do you think we'd be able to test this out using your second account, once merged?

Copy link
Contributor

@JamesMGreene JamesMGreene left a comment

Choose a reason for hiding this comment

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

This workflow looks good overall, though I left a few suggestions for best practices.

I don't see anything here that would warrant the need to run it in a separate account first, so just make sure to keep an eye out for potential failing runs in the future. 👍🏻

.github/workflows/welcome_message.yml Show resolved Hide resolved
types: [opened]
pull_request_target:
types: [opened]

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a best practice to restrict the permissions granted to the GITHUB_TOKEN to just the minimum set that are relevant to the workflow/job:

Suggested change
permissions:
issues: write
pull-requests: write

runs-on: ubuntu-latest
steps:
- name: 'Greet the contributor'
uses: garg3133/welcome-new-contributors@v1.2
Copy link
Contributor

Choose a reason for hiding this comment

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

From a security perspective, I would recommend you pin to a specific commit of the Action to ensure its tag (e.g. v1.2) is not maliciously swapped out from under you without giving you the opportunity to review it:

Suggested change
uses: garg3133/welcome-new-contributors@v1.2
uses: garg3133/welcome-new-contributors@a38583ed8282e23d63d7bf919ca2d9fb95300ca6

Copy link
Member

@paramsiddharth paramsiddharth left a comment

Choose a reason for hiding this comment

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

@Alex-is-Gonzalez I think it's best we move forward with James's advice and update the workflow. :)

@Alex-is-Gonzalez
Copy link
Collaborator Author

@JamesMGreene Thank you for the thorough feedback! I learned a lot from this, I went ahead and changed the yml based on the notes provided!

Copy link
Contributor

@JamesMGreene JamesMGreene left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for following up on my suggestions! ❤️

@JamesMGreene JamesMGreene merged commit 508ed62 into main Mar 19, 2023
@JamesMGreene JamesMGreene deleted the alex-is-gonzalez-wfb branch March 19, 2023 14:45
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.

None yet

3 participants