Reenable lint check in PR workflow and fix lint errors

[abhcs]

Description

Fix #1006 by using the right flags for lint. The lint action prettier -c was used to check for errors but in #902 was changed to prettier -w which does not do anything in the context of the PR workflow. We switch back to using -c again, and use fix instead of lint in the Makefile.

[abhcs]

See the error in the action for the above commit. Lint errors are now detected correctly.

[abhcs]

See the action details for the above commit:

Checking formatting...
All matched files use Prettier code style!

[anth-volk]

The reason we had originally changed prettier -c to prettier --write, though, was that numerous users would fail the test run and have to either manually sift through prettier's checks and fix stuff or know to run prettier --write - considering that we have quite a few contributors who are new to development, I don't know that that's the most intuitive workflow.

What if we split the difference by creating two make rules:

• make format_check could be run by the GitHub Action and consist of the call to black and prettier -c
• make format could be required of the user and run black and prettier --write

These could also be named differently

[abhcs]

I have not changed what make format does. It still writes. Please check again.

[anth-volk]

Apologies, I gave too cursory a read to your initial comments. Even so, could you change the format rule to something like make format-check and run your edited npm run lint there? This would ensure that the actions only check the formatting, not update it. Separately, I think it'd be best to have a make format that runs your new npm run fix that a user would run prior to opening a PR.

[anth-volk]

Since the linting issue we're having is that the actions lint prior to deploying, but don't actually alter the files as they sit in the repo

[abhcs]

I am not sure I understand which issue make format-check fixes -- which workflow uses this command?

[abhcs]

I am guessing that it is meant as an aid to new users?

[anth-volk]

On your first comment, I would envision the PR Action also being updated to use make format-check instead of make format, if you'd be willing to change it.

On the second, it's meant to do three things: be an aid, ensure that users can run both prettier and black in one action, and ensure that users can run prettier --write without the workflow running it, since the workflow's changes don't appear to affect the live repo, only the deployed app.

[abhcs]

Users can run prettier --write and black in one action right now in make format unless I am missing something. That's what npm run fix does. I am happy to make any changes but I do wish to understand the logic so that I don't mess up.

[abhcs]

If I understand the workflows right now, prettier --write will only occur when the user explicitly says so locally. There is no automated prettier --write before deploy if my cursory reading of push.yml is correct.

[anth-volk]

You know what, you're right, I introduced way too much confusion to this. I was thinking of the action that occurs on the API side, where we actually do explicitly run make format, and I had confused that with the call to npm run lint in the app's workflow. You're totally right, as it's written right now, this would all work. Sorry about the confusion.

[anth-volk]

You know what, you're right, I introduced way too much confusion to this. I was thinking of the action that occurs on the API side, where we actually do explicitly run make format, and I had confused that with the call to npm run lint in the app's workflow. You're totally right, as it's written right now, this would all work. Sorry about the confusion.

[anth-volk]

Thanks for your contributions on this, @abhcs, and thanks for catching my mistake on the workflows