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

40 code formatting #223

Merged
merged 8 commits into from
Mar 2, 2022
Merged

40 code formatting #223

merged 8 commits into from
Mar 2, 2022

Conversation

pihme
Copy link
Contributor

@pihme pihme commented Mar 1, 2022

Description

  • add Idea settings to align with code formatting rules
  • Add markdown formatting
  • Refactor parent pom
  • Change CI to apply code formatting and commit changes if necessary

Related issues

closes #40

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually

Documentation:

  • Javadoc has been written
  • The documentation is updated

- bind spotless to process-sources phase
- add markdown formatting on level of parent project
- put java/pom formatting into the inheritable settings
(refactored steps to run outside of test matrix)
@pihme
Copy link
Contributor Author

pihme commented Mar 1, 2022

@remcowesterhoud Not so sure on the latest commits. Maybe we want to run the code formatting before we run the tests. But otherwise it doesn't really matter 🤔

So please let me know how you want the CI pipeline to work

@github-actions
Copy link

github-actions bot commented Mar 1, 2022

Unit Test Results

124 files  124 suites   5m 39s ⏱️
321 tests 321 ✔️ 0 💤 0
772 runs  772 ✔️ 0 💤 0

Results for commit f50843c.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@remcowesterhoud remcowesterhoud left a comment

Choose a reason for hiding this comment

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

Thanks @pihme! Looks great!

❓ I'm assuming the code styles you've added are the same ones we use in Zeebe?
🔧 Line 346 of the root pom still has a comment mentioning it's a Google code format plugin. This is not really accurate anymore. (I can't comment on this line for some reason 🤔)

pom.xml Outdated Show resolved Hide resolved
@remcowesterhoud
Copy link
Contributor

For the pipeline question you asked. I think we should run the code formatting before the tests. The formatting shouldn't break any code, but if they ever have a bug that does it'd be nice to catch it immediately in our tests.

Copy link
Contributor

@remcowesterhoud remcowesterhoud left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to merge 👍

.github/workflows/build-test.yml Show resolved Hide resolved
@pihme pihme merged commit ad943c1 into main Mar 2, 2022
@pihme pihme deleted the 40-code-formatting branch March 2, 2022 13:40
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.

Configure IntelliJ to use identical formatting rules
2 participants