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
Jenkins: Present most recent interesting build info #8444
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8444 +/- ##
==========================================
- Coverage 54.75% 54.74% -0.02%
==========================================
Files 898 898
Lines 64251 64251
Branches 11440 11440
==========================================
- Hits 35179 35171 -8
- Misses 26390 26394 +4
- Partials 2682 2686 +4
Flags with carried forward coverage won't be shown. Click here to find out more. |
{ | ||
public string Url { get; set; } | ||
public long Timestamp { get; set; } | ||
public IEnumerable<JToken> JobDescription { get; set; } | ||
} | ||
|
||
public class JenkinsCacheInfo | ||
private class JenkinsCacheInfo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the class at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it stores the latest known build time for a project url. The adapter first asks for last build time, then requests the full data with all builds if there is anything new (that is more to save the server than the processing in the plugin).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you maybe meant why use a class, why not long directly?
I will change to that
Making a few other touchups, want to test this on Monday/Tuesday before pshing and initiates a merge
Plugins/BuildServerIntegration/JenkinsIntegration/JenkinsAdapter.cs
Outdated
Show resolved
Hide resolved
d5bb473
to
6acfc96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
6acfc96
to
6d67203
Compare
It takes time to test when the dev machine is accessible at night and test by day. |
6d67203
to
84370a7
Compare
Plugins/BuildServerIntegration/JenkinsIntegration/JenkinsAdapter.cs
Outdated
Show resolved
Hide resolved
84370a7
to
0d97431
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
0d97431
to
06e922f
Compare
@msftbot merge in 24 hours |
Hello @gerhardol! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
Jenkins refactoring and Re# suggestions
06e922f
to
4341f17
Compare
Proposed changes
Jenkins can start many builds for a commit, for instance for the branch and the pull request.
In some occasions the subsequent build(s) are aborted immediately, getting status NOT_BUILT
It is of course possible to abort builds too
The Jenkins build adapter presents the status for the latest build started for a commit regardless if it is complete or not.
This tries to present the most interesting status instead.
Test methodology
Manual
✒️ I contribute this code under The Developer Certificate of Origin.