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

added frontend formatting check and stand. steps #324

Merged
merged 2 commits into from
Aug 31, 2023

Conversation

to-sta
Copy link
Collaborator

@to-sta to-sta commented Aug 31, 2023

Contributor checklist


Description

This PR adds a github workflow to check frontend formatting with prettier. The workflow file pr_frontend_formatting_check.yaml was added. It targets the frontend folder and checks the formatting. Files are not changed but this can be changed in the long run with dry: False.

Similar to #316 i tried to use act to run the GitHub action locally but ran into a known issue.

Additional:
I standardize the steps inside pr_python_formatting_check.yaml. Minor change that makes the github actions logs more readable. We have to check this with a PR again 😃.

Suggestions:
I named the file pr_frontend_formatting_check could also be pr_prettier_formatting_check (or something else 😄) but i do like the frontend and backend distinction and would suggest to rename pr_python_formatting_check to pr_backend_formatting_check.

Related issue

@netlify
Copy link

netlify bot commented Aug 31, 2023

Deploy Preview for activist-org canceled.

Name Link
🔨 Latest commit 4170b3f
🔍 Latest deploy log https://app.netlify.com/sites/activist-org/deploys/64f0b1f13b4a500008a879e3

@github-actions
Copy link
Contributor

github-actions bot commented Aug 31, 2023

Thank you for the pull request!

The activist team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Development rooms once you're in. It'd be great to have you!

Maintainer checklist

  • The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution

    • The contributor's name and icon in remote commits should be the same as what appears in the PR
    • If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for git config user.email in their local activist repo
  • The TypeScript and formatting workflows within the PR checks do not indicate new errors in the files changed

  • The CHANGELOG has been updated with a description of the changes for the upcoming release (if necessary)

@andrewtavis andrewtavis self-requested a review August 31, 2023 14:58
@andrewtavis
Copy link
Member

Hey @to-sta! Thanks for this :) Glad to see my backend formatting worked out 😅🙃

I agree that we should rename the Python formatting check to backend 😊 If there are further languages/technologies added to either that make us need to expand the checks you’ve written, then we’d want them names frontend and backend anyway :)

Happy to merge once the rename is sent through and we’ll see on the next PR!

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me, @to-sta! Thanks for setting all this up 😊 As before let's wait for the next PR and then we can judge how it's all working and close the issue 🚀

I'm of the opinion that we should keep it at dry: True, but let me know what your thoughts are. To me this is a simple community responsibility where if it's not formatted we ask them to install the needed formatters, which further will help them contribute in the future via an appropriate environment. Unformatted code also would be an indication that there are other problems in the person's setup, so it gives us an opportunity to check in :)

@andrewtavis andrewtavis merged commit ff413a6 into activist-org:main Aug 31, 2023
4 checks passed
@to-sta
Copy link
Collaborator Author

to-sta commented Aug 31, 2023

@andrewtavis just checked the GitHub workflow on my side and made minor changes.
The working-directory key value pair did not work as intended but we can also use the prettier_options to set this up correctly.

jobs:
  prettier:
    name: Run PR Frontend Formatting Check
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v3
      - name: Run Prettier
        uses: creyD/prettier_action@v4.3
        with:
          dry: True
          prettier_options: ./frontend --check

I do agree on keeping dry: True since this is a not a big issue. Wrongly formatted code won't break the build 😄. Just mention this because i saw this in the styleguide:

...We'll eventually set it up so that code is autoformatted within the PR flow...

@andrewtavis
Copy link
Member

I'll correct this and remove the line above, @to-sta!

@to-sta to-sta mentioned this pull request Sep 1, 2023
1 task
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.

2 participants