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

Add link to reviewdog-action-prettier #12648

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

EPMatt
Copy link

@EPMatt EPMatt commented Apr 12, 2022

Description

Hi there,

first of all I'd really like to thank you for such an amazing code formatter. It has proven to be life-saving for me - I'm using it in many of my open source projects and it has always helped me keeping my codebases clean and properly formatted! 🚀

I recently developed reviewdog-action-prettier, a project which allows to run Prettier as an action in GitHub Actions CI/CD workflows. The action is based on reviewdog and provides both code annotations and suggestions as GitHub checks. You can find the documentation, as well as several examples of the tool in action, on the project's repo.

This PR adds a link to reviewdog-action-prettier in the Related Projects section in Prettier docs website.

Thank you so much for your precious time, and keep up with the amazing work!

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@prettier prettier deleted a comment from Harry-Hopkinson Apr 17, 2022
@prettier prettier deleted a comment from Harry-Hopkinson Apr 17, 2022
@fisker
Copy link
Member

fisker commented Apr 17, 2022

I'm not sure if we want add this link.

If I understand correctly, this tool install Prettier from npm then run instead of running existing script in package.json.

Personlly I don't like running Prettier in this way, it always running the latest version, and not easy to config options/patterns/ignore.

We should not encourage people use Prettier in this way.

(Just my own opinion.)

@EPMatt
Copy link
Author

EPMatt commented Apr 17, 2022

Hi @fisker,

first of all thank you for providing your opinion on this.

If I understand correctly, this tool install Prettier from npm then run instead of running existing script in package.json.

Not exactly. The action runs the following steps:

  • Checks whether Prettier has been already installed in the environment (npm bin);
  • If Prettier cannot be found, it looks for a package.json file with prettier declared as a dependency in the provided workdir, and installs the declared dependencies with npm install;
  • Runs Prettier with the provided prettier_flags. Of course configuration files can be used as well;
  • Runs reviewdog, which only takes care of analyzing Prettier's output, interacting with GitHub and creating the necessary checks and code annotations for the Pull Request/commit the action is run on.

it always running the latest version, and not easy to config options/patterns/ignore.

That's not exactly true. The action installs Prettier only if it cannot be found in the environment, and only if it's declared as a dependency in the package.json file. Therefore, the user has full control of the exact Prettier version which should be used by the action, and can also provide configuration both with CLI flags (via the prettier_flags input) and Prettier config files.
Moreover, if Prettier is not declared as a dependency or already installed, the action will result in an error.

I hope this better explains what this tool is about and how it interacts with the code formatter. Please let me know if you have any additional questions or doubts. :)
Thank you for your precious time!

@fisker
Copy link
Member

fisker commented Apr 20, 2022

Thanks for the explanation, I guess I was wrong.
This is good to me then.

Have you considered just config one command? In our codebase the config will be like

{
	"prettier_command": "yarn lint:prettier"
}

@EPMatt
Copy link
Author

EPMatt commented Apr 22, 2022

Hi @fisker,

thank you so much for your answer.

Have you considered just config one command?

Yeah, actually this is something that I had setup within my CI prior to developing this action. Then I bumped into the reviewdog project and I was stunned by the rich features it provides when integrated with compatible CI/CD services.

Just to provide additional context, the opening sentence from reviewdog's README better explains what the project is about:

reviewdog provides a way to post review comments to code hosting service, such as GitHub, automatically by integrating with any linter tools with ease. It uses an output of lint tools and posts them as a comment if findings are in diff of patches to review.

Having a single command configured in package.json, then running it manually within the CI eg. via npm run would only provide a yes/no answer to the question "is my codebase properly formatted?" - but it would not provide any useful information regarding which files and lines are affected.

In contrast, the main advantage of using reviewdog to run prettier and analyze its output (as reviewdog-action-prettier does) is that it provides rich code annotations and suggestions directly in the GitHub UI. Here are some screenshots which show how the output of this action looks within the GitHub Check report and PR UI respectively:

pr-check
pr-review

You can also check out this sample PR to see a full example usage of this tool.

I really enjoyed developing this GitHub action and I believe it could be extremely helpful for anyone trying to run prettier within their GitHub CI - as I'm currently doing in many of my repos. :)

Please let me know if you have any additional questions or doubts, I'd be glad to answer.

Thanks!

@fisker
Copy link
Member

fisker commented Apr 22, 2022

We try to add similar tool before #9057

@EPMatt
Copy link
Author

EPMatt commented May 9, 2022

Hi @fisker,

sorry for my late response. I can see that in the referenced PR you tried to implement some logic to auto fix code style issues.
The action I developed does not take care of that - it can suggest code changes, but not auto commit them. This is because in my opinion this feature does not align with the goal of the reviewdog ecosystem - providing code annotations, without automatically modifying the code.
Moreover, there's another github action which implements exactly that: https://github.com/creyD/prettier_action

Anyway, I'd be glad to help in setting up the Code Analysis CI for this repository, if you'd like to. We could use several actions from the reviewdog ecosystem, depending on which checks should be implemented for the codebase. Please let me know what are your thoughts about it. :)

Is any further action required from my side before merging this PR into main?

Thank you for your precious time!

@fisker
Copy link
Member

fisker commented May 10, 2022

https://github.com/creyD/prettier_action#usage, this is what I'm takings about, bad practice, I don't want define prettier version, patterns, flags again, we need something can run existing command.

@fisker
Copy link
Member

fisker commented May 10, 2022

Is any further action required from my side before merging this PR into main?

Nope, we need more review.

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

2 participants