Skip to content
This repository has been archived by the owner on May 7, 2024. It is now read-only.

Action checkstyle #43

Merged
merged 11 commits into from
Jun 5, 2021
Merged

Action checkstyle #43

merged 11 commits into from
Jun 5, 2021

Conversation

N0W0RK
Copy link
Owner

@N0W0RK N0W0RK commented Jun 1, 2021

Structure changes in .github.

Added GithubAction for CI using checkstyle with the Artemis configuration (closes #38)

@N0W0RK N0W0RK requested a review from CrsiX June 1, 2021 15:20
@JohannesStoehr
Copy link

Believe me, you want to leave out <module name="MagicNumber"/>.
Otherwise you will have a hard time writing the tests

@CrsiX
Copy link
Collaborator

CrsiX commented Jun 1, 2021

Based on GitHub Docs the issue and PR template files may not be recognized correctly. For example, valid path names for the PR template seem to be:

  • /pull_request_template.md
  • /docs/pull_request_template.md
  • /.github/pull_request_template.md
  • /.github/PULL_REQUEST_TEMPLATE/*

I think we should redo the renaming of the template files.

@CrsiX
Copy link
Collaborator

CrsiX commented Jun 1, 2021

I will add some more changes based on this document and I would really appreciate if you comment on them later on again, too. This way, we come to a solution together :)

@CrsiX
Copy link
Collaborator

CrsiX commented Jun 1, 2021

Looks like we will have a lot of cleaning work to do if we really want to get a 'good' code base. Reviewdog reports:

Filtered Findings (11973)

The previous setting was restricted to pushes or pull requests to
the main branch. Furthermore, 'workflow_dispatch' has been removed.
@CrsiX
Copy link
Collaborator

CrsiX commented Jun 1, 2021

We should also discuss about the setting of the fail_on_error value. Currently, it's false (default), meaning the tests will always pass with a ✔️ even if errors have been found. Otherwise (true), they would fail with a ❌ sign.
And finally, we could also add a badge to our README to show all users looking at the GitHub page that we have java style checkers in place (and they actually reported tons of style errors) ...

@JohannesStoehr
Copy link

JohannesStoehr commented Jun 1, 2021

Looks like we will have a lot of cleaning work to do if we really want to get a 'good' code base. Reviewdog reports:

Filtered Findings (11973)

:monkaGiga:
But a lot of errors seem to come from too long lines. Maybe running a linter once over every file would already fix a lot of these reported errors?

@N0W0RK
Copy link
Owner Author

N0W0RK commented Jun 2, 2021

First of all sory for fucking up the template location and configuration.

As far as I care, at least the line length case can be, either commented out or removed completely. We are creating sometimes large arrays with specific values as test algortithms would present a solution.

Failing on Error sounds like a good idea. Having Tests that dont compile does not sound reasonable to me.

We might also unify the names for the actions. I think there are 2/3 different names in the config.

Copy link
Collaborator

@CrsiX CrsiX left a comment

Choose a reason for hiding this comment

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

I would be fine with this settings now. Fine grained tuning can be done later on, too. Any other suggestions or comments on that? @N0W0RK

@N0W0RK N0W0RK merged commit 2799b5d into main Jun 5, 2021
@N0W0RK N0W0RK deleted the action-checkstyle branch June 5, 2021 13:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a CI pipeline to check for build errors and Java style guide violations
3 participants