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

Resolve issue from PR #397 and overall improve tests #403

Merged

Conversation

jluech
Copy link
Contributor

@jluech jluech commented May 17, 2021

  • Resolved issue from PR #397:
    • added extensive tests for shapeIntersects() method,
    • added different behavior for Line2D (a line does not have a geometric area),
    • switch methods to agreed solution mentioned in comments
  • Overall improved tests and increased coverage

@jluech
Copy link
Contributor Author

jluech commented May 17, 2021

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':sonarqube'.
> You're not authorized to run analysis. Please contact the project administrator.

Is that an issue we should fix or is it a configuration error on your side?

@nightm4re94
Copy link
Member

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':sonarqube'.
> You're not authorized to run analysis. Please contact the project administrator.

Is that an issue we should fix or is it a configuration error on your side?

My bad! We migrated our build pipeline from travis to GitHub actions this weekend. Need to exclude the sonar scan for builds outside the main repo again.

@jluech
Copy link
Contributor Author

jluech commented May 28, 2021

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':sonarqube'.
> You're not authorized to run analysis. Please contact the project administrator.

Is that an issue we should fix or is it a configuration error on your side?

My bad! We migrated our build pipeline from travis to GitHub actions this weekend. Need to exclude the sonar scan for builds outside the main repo again.

any news on that?

@nightm4re94
Copy link
Member

I've added a condition on the master branch that skips sonar scans in forks. please try merging and we'll see if it works as intended :)

@jluech
Copy link
Contributor Author

jluech commented May 30, 2021

I triggered a rerun of the checks, now "Gradle SonarQube Scan" is skipped as intended and all checks pass.
EDIT: at least here in the PR. In my fork repository, however, the master branch now also includes your change yet it starts the Java 8 build which will still fail on the sonarqube step with above error message...

I cannot merge without write access to the main repository though ;)

Copy link
Member

@nightm4re94 nightm4re94 left a comment

Choose a reason for hiding this comment

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

I'm sorry it took me this long to review. We're very grateful for your work, thank you!

@nightm4re94 nightm4re94 merged commit 20a8d09 into gurkenlabs:master Jun 8, 2021
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.

None yet

4 participants