errata-ai / vale-action

:octocat: The official GitHub Action for Vale -- install, manage, and run Vale with ease.
MIT License
201 stars 51 forks source link

Bump reviewdog to 0.20.2 #132

Closed anaxite closed 4 days ago

anaxite commented 1 month ago

Update vale-action to use reviewdog 0.20.2:

Resolves errata-ai/vale-action#131

SMoraisAnsys commented 3 weeks ago

@anaxite Could you have a look at my review ? I'm eager to be able to use this :D

SMoraisAnsys commented 2 weeks ago

@jdkato This PR seems to be stuck. Could you review it and add my add my comments if they make sense ? If required I can open a new PR and cherry-pick everything + commit my changes.

anaxite commented 2 weeks ago

Sorry for the wait!

How would it be best to approach compatibility? I tried to keep behavior the same when using the deprecated flag.

Alternately, this could be a good candidate for a new version of the action.

SMoraisAnsys commented 2 weeks ago

Sorry for the wait!

How would it be best to approach compatibility? I tried to keep behavior the same when using the deprecated flag.

Alternately, this could be a good candidate for a new version of the action.

I think you did well with respect to compatibility. Could you accept my changes if you agree with them or explain why you don't ? This would help the maintainer to have a PR ready to be reviewed.

anaxite commented 2 weeks ago

@SMoraisAnsys I've made changes, and fixed the README formatting while I was at it.

jdkato commented 2 weeks ago

Alternately, this could be a good candidate for a new version of the action.

This is a good idea.

Since a lot of users install the action using @reviewdog (instead of a specific version), I don't want to risk breaking their existing workflows by updating the default version of reviewdog.

So, when ready, I'll merge this into a new branch and put out a new tagged release.

anaxite commented 1 week ago

@SMoraisAnsys If this is going to go to another branch, then how about I remove the backwards compatibility?

SMoraisAnsys commented 1 week ago

That's a good point and a good idea IMO. However I'm not a maintainer so the final decision isn't mine :) What about removing the backward compatibility in a single commit and waiting for @jdkato feedback ? If he is fine with the final state then all right ! If not, a simple git revert SHA will do the trick.

anaxite commented 1 week ago

Thank you!

Aside from that last fix, I'm in favor of keeping backwards compatibility for the time being, and it can always be changed later. Same for the -level option.

SMoraisAnsys commented 5 days ago

@jdkato Sorry for the ping, just want to leverage this new feature :)

jdkato commented 4 days ago

You can try this out from the reviewdog20 branch now.