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

Initialize default view earlier in the init process. #8413

Merged
merged 5 commits into from
Aug 29, 2023

Conversation

Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Aug 22, 2023

Perform the default view initialization next to loading the configuration from disk.

This is more consistent as the primary view and views are cleared just before loading from disk, which leads to a temporary inconsistent state while Jenkins initializes.

See JENKINS-XXXXX.

Testing done

Proposed changelog entries

  • Developer: Initialize default view slightly earlier in the initialization process.

Proposed upgrade guidelines

N/A

Submitter checklist

Preview Give feedback

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Preview Give feedback

Perform the default view initialization next to loading the
configuration from disk.

This is more consistent as the primary view and views are cleared just
before loading from disk, which leads to a temporary inconsistent state while
Jenkins initializes.
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Seems like a more logical spot to do this.

core/src/main/java/jenkins/model/Jenkins.java Outdated Show resolved Hide resolved
@Vlatombe Vlatombe marked this pull request as ready for review August 24, 2023 12:42
@Vlatombe Vlatombe requested a review from jglick August 24, 2023 12:44
jglick

This comment was marked as resolved.

@NotMyFault NotMyFault added the developer Changes which impact plugin developers label Aug 28, 2023
@NotMyFault NotMyFault requested a review from a team August 28, 2023 06:31
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

/label ready-for-merge


This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Aug 28, 2023
@NotMyFault NotMyFault merged commit 7b1dfd8 into jenkinsci:master Aug 29, 2023
@Vlatombe Vlatombe deleted the fix-all-view-init branch September 1, 2023 14:39
assertTrue(CheckInitialViewExtension.hasPrimaryView);
}

@TestExtension(value = "checkInitialView")
Copy link
Member

Choose a reason for hiding this comment

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

By the way I think this has no actual effect—this @Initializer will be run regardless of which test is running (all of jenkins-test).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer Changes which impact plugin developers ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants