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

General Repository Improvements #155

Merged
merged 48 commits into from
Apr 18, 2023
Merged

Conversation

GrantBirki
Copy link
Member

@GrantBirki GrantBirki commented Apr 13, 2023

General Repository Improvements

This pull request does the following:

  • Adds ~70% test coverage
  • Updates the readme
  • Adds status badges
  • Removes the duplicate CODEOWNERS file
  • Adds a coverage badge
  • Improves CI jobs
  • Adds all inputs and outputs for this Action to the main readme
  • Updates all node packages
  • Adds a CONTRIBUTING.md file for new contributors
  • Structures __tests__ and src/ dir with functions folder
  • Sprinkles a bit of debugging in a few places

What is a classic grantbirki driveby 🚗? This is where @GrantBirki swings by your repo and surprises you with a PR that helps ya out!

@GrantBirki GrantBirki added documentation Improvements or additions to documentation enhancement New feature or request dependencies Pull requests that update a dependency file labels Apr 13, 2023
@GrantBirki GrantBirki added the javascript Pull requests that update Javascript code label Apr 13, 2023
@GrantBirki GrantBirki self-assigned this Apr 13, 2023
__tests__/index.test.js Fixed Show fixed Hide fixed
__tests__/index.test.js Fixed Show fixed Hide fixed
@GrantBirki GrantBirki marked this pull request as ready for review April 14, 2023 20:32
@GrantBirki GrantBirki requested a review from a team as a code owner April 14, 2023 20:32

it('executes cleanly', done => {
const ip = path.join(__dirname, '../index.js')
cp.exec(`node ${ip}`, { env: process.env }, (err, stdout) => {

Check warning

Code scanning / CodeQL

Shell command built from environment values

This shell command depends on an uncontrolled [absolute path](1).
it('execution fails if there are missing variables', done => {
delete process.env.ACTIONS_RUNTIME_URL
const ip = path.join(__dirname, '../index.js')
cp.exec(`node ${ip}`, { env: process.env }, (err, stdout) => {

Check warning

Code scanning / CodeQL

Shell command built from environment values

This shell command depends on an uncontrolled [absolute path](1).
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.

Thanks for the contributions! Lots of great nuggets within this PR. 💝 😍

I've taken the liberty of rolling back some of the changes that didn't feel especially relevant or desirable. ⏪

In the future, I would encourage a handful of smaller PRs instead of one large PR, as that typically results in less effort needed to get the favorable bits merged. 🙏🏻 🙇🏻

@JamesMGreene JamesMGreene merged commit 176fcdb into main Apr 18, 2023
@JamesMGreene JamesMGreene deleted the classic-grantbirki-driveby branch April 18, 2023 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants