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

feat: Initial version #1

Merged
merged 21 commits into from
Jul 4, 2021
Merged

feat: Initial version #1

merged 21 commits into from
Jul 4, 2021

Conversation

oscard0m
Copy link
Owner

@oscard0m oscard0m commented Jul 2, 2021

Your new script is ready to be implemented 🎉

You can run it locally on your machine with

node cli.js

Once you are happy with it, push your changes to this pull request

git commit -a -m "feat: initial version"
git push

Before merging:


Issues to solve

Before first release

  • To not open a PR if there are no setup-node changes (https://github.com/oscard0m/web/pull/50/files?diff=unified&w=1)
  • If there is no .github/workflow dir, skip (i.e. {"name":"HttpError","status":404,"response":{"url":"https://api.github.com/repos/oscard0m/webpack-tutorial/contents/.github%2Fworkflows","status":404,"headers":{"access-control-allow-origin":"*","access-control-expose-headers":"ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, … })

After first release

@oscard0m
Copy link
Owner Author

oscard0m commented Jul 2, 2021

Context

setup-node GitHub Action just released a new option to add cache to setps using it.
Deteails here: https://github.blog/changelog/2021-07-02-github-actions-setup-node-now-supports-dependency-caching/

Mission

  • I would like to automate to add this cache option to all my GitHub Actions with setup-node
  • Do you think this could be interesting for Octokit and other orgs? @gr2m
  • I have implemented an initial version of an Octoherd script to add cache property to all the GitHub Actions using actions/setup-node. I did not test it exhaustively yet but I have an initial PR opened [EDIT 4/7/2021]: here here

Problems

I'm using js-yaml which it applies some extra changes which add a bit of noise of the PR. When parsing and dumping back the yaml file:

  • It does not preserve comments of the file
  • It does not preserve newlines as before
  • Transforms arrays to indented newlines

[EDIT 4/7/2021]: I come up with a solution with yaml library instead of js-yaml. Kudos to @eemeli. You can find details here

Review Request

@gr2m I would like you to take a look when you have time and, specially if you would see this valuable or not that important.

Thanks in advance


Most of the lines are inspired by @wolfy1339 in this repo: https://github.com/wolfy1339/octoherd-script-fix-prettier-update
I see some room to wrap some logic into @octokit plugins here.

@wolfy1339
Copy link

Can you pass it through prettier before writing the file? That could solve the issue with indentation

@oscard0m
Copy link
Owner Author

oscard0m commented Jul 3, 2021

Can you pass it through prettier before writing the file? That could solve the issue with indentation

Running npx prettier --parser yaml <file> --write does not convert sequences to arrays since they are both YAML valid and equivalent.

Tried using yaml library (the one used by prettier) and no luck neither. It seems I'm not the only one with this issue.

I'm not sure if there is a cool way to parse/dump yaml files and not lose data/format in the process. Posted an issue to js-yaml.


[EDIT 4/7/2021]: I made it work after asking for help in eemeli/yaml#220 (reply in thread)
🍾 🍾 🍾 (this solution implied running prettier too, thanks for that tip @wolfy1339)

@gr2m
Copy link

gr2m commented Jul 4, 2021

  • Do you think this could be interesting for Octokit and other orgs? @gr2m

Absolutely! I just talked about this exact thing with @travi

Copy link

@gr2m gr2m 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 creating this script! I think folks will love it!

README.md Outdated Show resolved Hide resolved
script.js Outdated Show resolved Hide resolved
script.js Outdated Show resolved Hide resolved
script.js Outdated Show resolved Hide resolved
script.js Outdated Show resolved Hide resolved
script.js Outdated Show resolved Hide resolved
script.js Outdated Show resolved Hide resolved
script.js Outdated Show resolved Hide resolved
script.js Outdated Show resolved Hide resolved
@oscard0m oscard0m merged commit f08d2b8 into main Jul 4, 2021
@oscard0m oscard0m deleted the initial-version branch July 4, 2021 23:20
@github-actions
Copy link

github-actions bot commented Jul 4, 2021

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants