imgix / imgix-java

A Java client library for generating URLs with imgix
https://www.imgix.com
BSD 2-Clause "Simplified" License
19 stars 8 forks source link

test: update junit syntax #66

Closed luqven closed 3 years ago

luqven commented 3 years ago

Description

This PR:

commit-lint[bot] commented 3 years ago

Tests

Contributors

luqven

Commit-Lint commands
You can trigger Commit-Lint actions by commenting on this PR: - `@Commit-Lint merge patch` will merge dependabot PR on "patch" versions (X.X.Y - Y change) - `@Commit-Lint merge minor` will merge dependabot PR on "minor" versions (X.Y.Y - Y change) - `@Commit-Lint merge major` will merge dependabot PR on "major" versions (Y.Y.Y - Y change) - `@Commit-Lint merge disable` will desactivate merge dependabot PR - `@Commit-Lint review` will approve dependabot PR - `@Commit-Lint stop review` will stop approve dependabot PR
luqven commented 3 years ago

Love the parameterized tests, would definitely like to see more of those in the future 👍 Also, thanks for including the descriptive comments, they were really helpful while going through the PR.

Nothing critical but I wanted to ask: I noticed there are a lot of whitespace changes here - was that done automatically by sonar-lint or your IDE? If the former, is there a way we can include a lint task in build.gradle other contributors. I'm mainly concerned with each of us accidentally overwriting these changes if we aren't set on using the same linter.

Great q, there's a high likelihood that's sonarlint's doing. What I can do is find a way to add these lint rules to a .vscode folder. But not all of us dev on Code so 🤷🏼 that might not be best. I'm leaning towards disabling sonarlint, since its not in IntelliJ. That should resolve these whitespace conflicts in future.

Bummer, since sonar tends to catch some neat stuff 😢 .

luqven commented 3 years ago

Update on the last comment.

Turns out SonarLint is on IntelliJ 🎉

~So on top of these Gradle tasks, we can also update the contributing docs to ensure people have it in their IDE.~

luqven commented 3 years ago

@sherwinski so the linting issues stem from the fact that IntelliJ doesn't really have a public API for linting rulesets. Any spacing changes in VsCode will conflict with those auto-formatted in IntelliJ unless we pick a standard that does have a format-api, like google-java-format.

So, I'm going to leave this PR as is but open up a follow up PR that addresses this issue by doing 2 things:

Anyone on IntelliJ will be able to follow the same linting rules thanks to the gradle tasks linting against the same format.