sethvargo/ratchet

Don't update anything other than the SHA

GrantBirki opened this issue · 4 comments

TL;DR

I am requesting a feature flag like --sha-only that only updates the SHA of the workflow file. Currently, it will change newlines, remove whitespaces, etc. This is undesirable as it can lead to a large and unnecessary diff for large workflow files

Detailed design

Ratchet will currently turn this:

on:
  issue_comment:
    types: [ created ]

into this:

on:
  issue_comment:
    types: [created]

It doesn't actually make a difference to how the workflow runs and it can lead to a large diff if it makes many whitespace changes throughout your workflow.

I think this feature is great in the sense that ratchet provides a level of linting but it would be great if there was an option to disable that and it would only change pinned versions of Actions.

Running ratchet on one of my larger workflow files lead to a ~300 line diff when nothing was actually changing apart from one workflow version being pinned. This could lead to troublesome peer reviews on pull requests with a huge whitespace diff and makes it hard to read.

❤️

Hi @GrantBirki

Thank you for opening an issue. This is documented under known issues at the bottom of the readme.

The library we use to parse the YAML into an AST tree does not preserve whitespace or indentation, so unfortunately there's nothing I can do here.

The original version of YAML did regex-based replacements on the file, but that proved to be extremely error-prone, so now it parses and re-renders the AST tree.

Sorry 😦

Hey @sethvargo, I'm experimenting with a script to copy just the pinned references over to the original file to preserve formatting so our automated ratchet tooling is as un-intrusive as possible for teams.

I was coming here to ask whether this is something you'd consider inside Ratchet itself, but seeing your last comment here means you'd likely warn us to retire the regexes and just do it properly? 😄

Hi @liamoneill yes - what you'll find is that a regex-based approach is insufficient because replacement requires context that can only be derived from the ast tree.

Thank @sethvargo, and thank you for the awesome tool as well.