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

[JENKINS-49729] Add some css to job and run pages for dead branches #151

Merged
merged 3 commits into from
May 10, 2019

Conversation

rsandell
Copy link
Member

@rsandell rsandell commented May 9, 2019

Also had to tweak the dependencies a bit since the introduction of
git 4.0.0-rc broke hpi:run

Before
Job Page
JENKINS-49729-BEFORE
Except for the "This job is currently disabled" message which was still there before

*Build Page*

JENKINS-49729-BEFORE-build

After
Job Page
JENKINS-49729-AFTER-job
Build Page
JENKINS-49729-AFTER-build

Also had to tweak the dependencies a bit since the introduction of
git 4.0.0-rc broke hpi:run
@rsandell
Copy link
Member Author

rsandell commented May 9, 2019

Added @jglick since I overwrote his git 4 change.

Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

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

Seems fine though I am a bit confused about the name of the Jelly file and it would be nice to see a screenshot.

/**
* An action that puts some css on job and run pages for jobs representing {@link Branch.Dead}.
*/
public class DeadBranchIndicatorAction implements Action {
Copy link
Member

Choose a reason for hiding this comment

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

FWIW there is hudson.model.InvisibleAction for this use case, although no harm in keeping it how it is now.

}
h1.job-index-headline {
text-decoration: line-through;
color: silver;
Copy link
Member

Choose a reason for hiding this comment

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

Before/after screenshots would be nice to see what it looks like.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout"
xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt">
<!--TODO build-caption i.e. the links to the builds on the job page doesn't take effect-->
<style type="text/css">
Copy link
Member

Choose a reason for hiding this comment

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

How does jobMain.jelly get loaded? I would've thought it would need to be named action.jelly to be picked up here, but I am probably missing something.

Copy link
Member Author

Choose a reason for hiding this comment

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

They are standard action views across both AbstractBuilds and WorkflowRuns.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is also floatingBox.jelly which renders the trend graphs on the right hand side of a build page.

The build summary worked for me when I tried at first because
there was an old summary.jelly in the cache from an earlier attempt.

Also use the actual disabled ball color instead of hardcoded silver.
And InvisibleAction instead of standard Action.
@rsandell
Copy link
Member Author

The build failures seems to be because problems with the windows agents getting terminated.
It builds fine locally for me, though I would like to see the Java 11 tests running but they don't get a chance to it seems.

@rsandell
Copy link
Member Author

Maybe I'm reading the build logs wrong, so gonna try to revert the pom changes to see if that helps.

@dwnusbaum
Copy link
Member

I think you were just running into infra issues, I've see this error on various repositories (I would link the relevant INFRA ticket but am getting a 502 on issues.jenkins-ci.org...):

win2012-828200 was marked offline: Connection was broken: java.util.concurrent.TimeoutException: Ping started at 1557486180934 hasn't completed by 1557486420934

Best option I know of in these cases is just to close/reopen the PR to get a fresh build until it passes.

@rsandell rsandell removed the request for review from jglick May 10, 2019 14:03
@rsandell
Copy link
Member Author

Well the revert helped (or the rerun) so I'll leave it as is unless someone else reviews and finds something :)

@rsandell rsandell merged commit 95bebc1 into master May 10, 2019
@rsandell rsandell deleted the JENKINS-49729 branch May 10, 2019 14:36
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.

2 participants