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

chore: add check_code_format.yml #27

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

vil02
Copy link
Member

@vil02 vil02 commented Sep 24, 2024

This PR adds a workflow, which checks the used code formatting (and makes the necessary changes in the currently existing files).

As suggested by @Ramy-Badr-Ahmed in #13, the new workflow uses fprettify. I could not force it to fail, if there are some changes necessary, so I used a fail if needs reformatting approach as in TheAlgorithms/Nim#19.

Beside the indentation size being set to 4, I used the default setting of the fprettify. Please let me know if you prefer other settings.

The failing part works logs for 8663d15.

@vil02 vil02 marked this pull request as ready for review September 24, 2024 20:24
@Ramy-Badr-Ahmed
Copy link
Member

Ramy-Badr-Ahmed commented Sep 24, 2024

Beside the indentation size being set to 4, I used the default setting of the fprettify. Please let me know if you prefer other settings.

The failing part works logs for 8663d15.

Adding the debug output -d to fprettify --indent 4 -d --recursive . can help identifiy the styling mismatches by contributors.

Here's an example output: logs

@vil02 vil02 marked this pull request as draft September 25, 2024 20:08
@vil02 vil02 force-pushed the add_check_format branch 3 times, most recently from 33d0ee6 to 08c604b Compare September 25, 2024 20:32
@vil02
Copy link
Member Author

vil02 commented Sep 25, 2024

Beside the indentation size being set to 4, I used the default setting of the fprettify. Please let me know if you prefer other settings.
The failing part works logs for 8663d15.

Adding the debug output -d to fprettify --indent 4 -d --recursive . can help identifiy the styling mismatches by contributors.

Here's an example output: logs

Strange... the output does not appear in the actions logs:

EDIT: of course the output is empty... the files are already reformatted. I will use git diff.

@vil02 vil02 marked this pull request as ready for review September 25, 2024 20:34
@Ramy-Badr-Ahmed
Copy link
Member

Ramy-Badr-Ahmed commented Sep 26, 2024

Strange... the output does not appear in the actions logs:
* https://github.com/TheAlgorithms/Fortran/actions/runs/11040556555/job/30668810319
* https://github.com/TheAlgorithms/Fortran/actions/runs/11040442388/job/30668440497
* https://github.com/TheAlgorithms/Fortran/actions/runs/11040275300/job/30667876416

I see. -d flag shows differences, no changes made.

I tried it out and it would work with git diff in the failure step:

      - name: Format Fortran files
        run: |          
          fprettify -i 4 --recursive .
      - name: Fail if needs reformatting      
        run: |  
          if [[ $(git status --porcelain) ]]; then            
            echo "please reformat/fprettify these files:"            
            git diff
            exit 1
          else
            echo "All files are properly formatted."
          fi   

My logs: https://github.com/Ramy-Badr-Ahmed/Fortran-DSA/actions/runs/11043111068/job/30676793615

Let's have another try 🙂

@vil02
Copy link
Member Author

vil02 commented Sep 26, 2024

So as mentioned above: the output was empty, because obviously the files were already reformatted. I used git diff.

Now it works as expected, cf. the failing case: https://github.com/TheAlgorithms/Fortran/actions/runs/11045781263/job/30683988597

Copy link
Member

@Ramy-Badr-Ahmed Ramy-Badr-Ahmed left a comment

Choose a reason for hiding this comment

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

Yes, it's now very informative 👍

.github/workflows/check_code_format.yml Show resolved Hide resolved
Copy link
Member

@SatinWukerORIG SatinWukerORIG left a comment

Choose a reason for hiding this comment

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

👍 !

@SatinWukerORIG SatinWukerORIG merged commit ea1245e into TheAlgorithms:main Sep 27, 2024
6 checks passed
@vil02 vil02 deleted the add_check_format branch September 27, 2024 06:37
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.

3 participants