Move configuration to appearance page #109

timja · 2023-08-24T21:16:21Z

This is a breaking change for JCasC configuration, it moves from unclassified to appearance, given this change had to happen anyway I took the liberty of using a more correct symbol of prism, as symbols are not supposed to include their extension point configuration was unneeded.

The new config is:

appearance:
  prism:
    theme: "DARK"
    sourceDirectories:
    - path: "C:\\Windows"
    - path: "/absolute"

Testing done

Submitter checklist

Give feedback

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue
Options

codecov · 2023-08-24T21:23:40Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (7b7a209) 75.13% compared to head (b28e5ac) 75.59%.
Report is 3 commits behind head on main.

❗ Current head b28e5ac differs from pull request most recent head a481376. Consider uploading reports for the commit a481376 to get more accurate results

Files	Patch %	Lines
...ns/plugins/prism/PrismAppearanceConfiguration.java	94.73%	0 Missing and 1 partial ⚠️
...a/io/jenkins/plugins/prism/PrismConfiguration.java	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #109      +/-   ##
============================================
+ Coverage     75.13%   75.59%   +0.46%     
- Complexity       87       91       +4     
============================================
  Files            12       13       +1     
  Lines           366      377      +11     
  Branches         30       30              
============================================
+ Hits            275      285      +10     
- Misses           88       89       +1     
  Partials          3        3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pom.xml

uhafner

Wouldn't it make more sense to move only the theme to the appearance section? The other option is more a security setting, letting administrators define allowed source code directories?

timja · 2023-09-15T13:08:32Z

Wouldn't it make more sense to move only the theme to the appearance section? The other option is more a security setting, letting administrators define allowed source code directories?

Yes that would be better (and is also what is in the guidance here https://javadoc.jenkins.io/jenkins/appearance/AppearanceCategory.html)

timja · 2023-10-08T19:02:16Z

This is ridiculous @uhafner

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:05 min
[INFO] Finished at: 2023-10-08T20:00:23+01:00
[INFO] ------------------------------------------------------------------------

uhafner · 2023-10-08T19:23:04Z

This is ridiculous @uhafner

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:05 min
[INFO] Finished at: 2023-10-08T20:00:23+01:00
[INFO] ------------------------------------------------------------------------

Don't blame me for this concept. Maven does only support success or failure. All Java analysis tools and javac do not fail the build with purpose on warnings. When everything is a failure then Jenkins does not finish processing, this is also a problem. Seems that you are using a totally different development workflow. I use maven command line very rarely. I run static analysis and tests within the IDE before each commit. I use maven only to deploy and release my plugins.

uhafner

Thanks! Moving the source directories below the security category makes sense as well.

uhafner · 2023-10-12T15:53:40Z

pom.xml

    <changelist>-SNAPSHOT</changelist>
+    <jenkins.version>2.421</jenkins.version>


If this is ok for you I will not merge until 2.426.1 is ready.

uhafner · 2023-10-12T15:55:11Z

pom.xml


    <module.name>${project.groupId}.prism</module.name>

    <testcontainers.version>1.19.0</testcontainers.version>
    <jsoup.version>1.16.1</jsoup.version>
-
+    <hpi.compatibleSinceVersion>1.30.0-1</hpi.compatibleSinceVersion>


Does this work with a number of 1.29.0-9 as well? I want to keep the first part of the version number in sync with the version number of Prism.js.

yes it will be fine

uhafner · 2023-10-12T16:06:59Z

src/main/java/io/jenkins/plugins/prism/PrismAppearanceConfiguration.java

+ * @author Ullrich Hafner
+ */
+@Extension
+@Symbol("prism")


Is it ok to use the same ID twice? In steps this should be avoided buy maybe here it is ok?

it's ok as it's in a different category.

Normally it's extension points that shouldn't have the same one but as far as I know as long as it's in a different category it should be fine.

timja · 2023-11-16T09:07:44Z

possible to relook at this @uhafner?

pom.xml

uhafner · 2023-11-17T22:14:07Z

Yes, I'll merge it after the weekend...

Move configuration to appearance page

e614e99

timja added the enhancement Enhancement of existing functionality label Aug 24, 2023

timja mentioned this pull request Aug 24, 2023

Add Appearance system configuration page jenkinsci/jenkins#8403

Merged

timja commented Sep 10, 2023

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

Update pom.xml

db10f9f

timja marked this pull request as ready for review September 10, 2023 21:22

uhafner reviewed Sep 15, 2023

View reviewed changes

Merge branch 'main' into appearance-page

c3ff386

uhafner changed the title ~~Move configuration to appearance page~~ [LTS Jenkins 2.426] Move configuration to appearance page Oct 5, 2023

uhafner marked this pull request as draft October 5, 2023 06:20

Split appearance and permitted source directories

18c677c

Maybe fix the build based off of what the checks are saying

62ef47b

timja requested a review from uhafner October 8, 2023 19:14

uhafner approved these changes Oct 12, 2023

View reviewed changes

Merge branch 'main' into appearance-page

6ae3393

timja commented Nov 16, 2023

View reviewed changes

pom.xml Show resolved Hide resolved

Update pom.xml

b28e5ac

uhafner marked this pull request as ready for review November 21, 2023 08:21

Set Jenkins baseline to 2.426.1 LTS.

a481376

github-actions bot assigned uhafner Nov 21, 2023

uhafner changed the title ~~[LTS Jenkins 2.426] Move configuration to appearance page~~ Move configuration to appearance page Nov 21, 2023

uhafner merged commit 424a8f2 into jenkinsci:main Nov 21, 2023
25 checks passed

timja deleted the appearance-page branch November 21, 2023 14:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move configuration to appearance page #109

Move configuration to appearance page #109

timja commented Aug 24, 2023 •

edited

Loading

Submitter checklist

codecov bot commented Aug 24, 2023 •

edited

Loading

uhafner left a comment

timja commented Sep 15, 2023

timja commented Oct 8, 2023

uhafner commented Oct 8, 2023

uhafner left a comment

uhafner Oct 12, 2023

timja Oct 13, 2023

uhafner Oct 12, 2023

timja Oct 13, 2023

uhafner Oct 12, 2023

timja Oct 13, 2023 •

edited

Loading

timja commented Nov 16, 2023

uhafner commented Nov 17, 2023

		<changelist>-SNAPSHOT</changelist>
		<jenkins.version>2.421</jenkins.version>

Move configuration to appearance page #109

Move configuration to appearance page #109

Conversation

timja commented Aug 24, 2023 • edited Loading

Testing done

Submitter checklist

codecov bot commented Aug 24, 2023 • edited Loading

Codecov Report

uhafner left a comment

Choose a reason for hiding this comment

timja commented Sep 15, 2023

timja commented Oct 8, 2023

uhafner commented Oct 8, 2023

uhafner left a comment

Choose a reason for hiding this comment

uhafner Oct 12, 2023

Choose a reason for hiding this comment

timja Oct 13, 2023

Choose a reason for hiding this comment

uhafner Oct 12, 2023

Choose a reason for hiding this comment

timja Oct 13, 2023

Choose a reason for hiding this comment

uhafner Oct 12, 2023

Choose a reason for hiding this comment

timja Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

timja commented Nov 16, 2023

uhafner commented Nov 17, 2023

timja commented Aug 24, 2023 •

edited

Loading

codecov bot commented Aug 24, 2023 •

edited

Loading

timja Oct 13, 2023 •

edited

Loading