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

Add timeout to main build (#322) #323

Merged
merged 1 commit into from
Aug 17, 2020
Merged

Add timeout to main build (#322) #323

merged 1 commit into from
Aug 17, 2020

Conversation

beatngu13
Copy link
Member

@beatngu13 beatngu13 commented Aug 16, 2020

Proposed commit message:

Add timeout to main build (#322)

According to #322, the main build occasionally times out. Until
further investigation, this PR will add a timeout to the main build.

PR: #323

Note that I didn't close the corresponding issue on purpose. Maybe we want to rephrase it because I think we are better off investigating why the build actually runs forever. The added timeout only addresses a sympton, not the disease.


PR checklist

The following checklist shall help the PR's author, the reviewers and maintainers to ensure the quality of this project.
It is based on our contributors guidelines, especially the "writing code" section.
It shall help to check for completion of the listed points.
If a point does not apply to the given PR's changes, the corresponding entry can be simply marked as done.

Documentation (general)

  • There is documentation (Javadoc and site documentation; added or updated)
  • There is implementation information to describe why a non-obvious source code / solution got implemented
  • Site documentation has its own .adoc file in the docs folder, e.g. docs/report-entries.adoc
  • Only one sentence per line (especially in .adoc files)
  • Javadoc uses formal style, while sites documentation may use informal style (see Documentation: Personal or formal style #265)

Documentation (new extension)

  • The docs/docs-nav.yml navigation has an entry for the new extension
  • The package-info.java contains information about the new extension

Code

  • Code adheres to code style, naming conventions etc.
  • Successful tests cover all changes
  • There are checks which validate correct / false usage / configuration of a functionality and there are tests to verify those checks (see Require configuration failiure tests? #164)
  • Tests use AssertJ or our own PioneerAssert (which are based on AssertJ)

Contributing

  • A prepared commit message exists
  • The list of contributions inside README.md mentions the new contribution (real name optional)

I hereby agree to the terms of the JUnit Pioneer Contributor License Agreement.

@sonarcloud
Copy link

sonarcloud bot commented Aug 16, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@Bukama
Copy link
Member

Bukama commented Aug 16, 2020

Note that I didn't close the corresponding issue on purpose. Maybe we want to rephrase it because I think we are better off investigating why the build actually runs forever. The added timeout only addresses a sympton, not the disease.

I think it's okay to do this seperated and first kill the sympton to don't block resources too long. I think to define a well chosen timeout even when there are no known issues is fine. Meaning we can and should close the issue ans the issue is about defining a timeout. Investigate and fix the issue of the timeouting build is another thing and we should not mix issues.

Copy link
Member

@aepfli aepfli left a comment

Choose a reason for hiding this comment

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

LGTM

@aepfli
Copy link
Member

aepfli commented Aug 16, 2020

i do understand both sides regarding issue, and i am fine with both - so @Bukama and @beatngu13 as soon as you agreed on that, either merge or let me know and i will merge :)

@beatngu13
Copy link
Member Author

I'm all good with @Bukama's suggestions. Well-chosen timeouts are indeed sensible*, I just wanted to emphasize that we have to dig deeper to find out what's actually the issue. I don't care if we create one, two or more issues for that. ✌️

*: Should we add them to the other workflows as well?

@Bukama Bukama merged commit b0ad64a into junit-pioneer:master Aug 17, 2020
@beatngu13 beatngu13 deleted the patch-1 branch August 17, 2020 19:03
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

3 participants