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 spotless run and apply to all files #715

Merged
merged 13 commits into from
Oct 5, 2023
Merged

Conversation

olamy
Copy link
Member

@olamy olamy commented Oct 3, 2023

No description provided.

@olamy olamy requested a review from a team as a code owner October 3, 2023 04:31
@olamy
Copy link
Member Author

olamy commented Oct 3, 2023

@bguerin I know this is changing a lot of files. but no choice to make this incremental :)

pipeline-maven/pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@bguerin bguerin self-assigned this Oct 3, 2023
@bguerin bguerin added the chore label Oct 3, 2023
@bguerin
Copy link
Contributor

bguerin commented Oct 4, 2023

Also, if POMs are now handle by spotless plugin, no need for sortpom-maven-plugin anymore (lines 259 ~ 275 and corresponding property line 110)

@bguerin
Copy link
Contributor

bguerin commented Oct 4, 2023

FYI build is failing because the module pipeline-maven does a copy of the spy artefact. This worked because of the module declaration order. I think we should explicit this dependency (but in provided ? scope, to avoid problems with hpi packaging)

@olamy
Copy link
Member Author

olamy commented Oct 4, 2023

yup sorry I have been busy with something else. it should be fixed now.

Signed-off-by: Olivier Lamy <[email protected]>
@@ -288,7 +287,8 @@
}

@Restricted(NoExternalUse.class) // Only for UI calls
public ListBoxModel doFillGlobalMavenSettingsConfigItems(@AncestorInPath Item item, @AncestorInPath ItemGroup context) {
public ListBoxModel doFillGlobalMavenSettingsConfigItems(

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If DescriptorImpl#doFillGlobalMavenSettingsConfigItems connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
@Restricted(NoExternalUse.class) // Only for UI calls
public ListBoxModel doFillMavenSettingsConfigItems(@AncestorInPath Item item, @AncestorInPath ItemGroup context) {
public ListBoxModel doFillMavenSettingsConfigItems(

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If DescriptorImpl#doFillMavenSettingsConfigItems connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
@bguerin
Copy link
Contributor

bguerin commented Oct 4, 2023

Line 110 of the main POM (<maven-plugin-sortpom.version>3.3.0</maven-plugin-sortpom.version>) can be removed ;)

Signed-off-by: Olivier Lamy <[email protected]>
@olamy
Copy link
Member Author

olamy commented Oct 5, 2023

it looks to fail on Windows because files were initially wrongly created....
no much time to look at this before next week.

@bguerin
Copy link
Contributor

bguerin commented Oct 5, 2023

it looks to fail on Windows because files were initially wrongly created.... no much time to look at this before next week.

No worry, I understand, thanks a lot for all you work until here

Maybe you could grant me write access to you branch so I can end and merge this. I would like to merge this before others

@olamy
Copy link
Member Author

olamy commented Oct 5, 2023

it looks to fail on Windows because files were initially wrongly created.... no much time to look at this before next week.

No worry, I understand, thanks a lot for all you work until here

Maybe you could grant me write access to you branch so I can end and merge this. I would like to merge this before others

you should already have access. it's default.
screenshot
Screenshot from 2023-10-05 16-47-51

@bguerin
Copy link
Contributor

bguerin commented Oct 5, 2023

it looks to fail on Windows because files were initially wrongly created.... no much time to look at this before next week.

No worry, I understand, thanks a lot for all you work until here
Maybe you could grant me write access to you branch so I can end and merge this. I would like to merge this before others

you should already have access. it's default. screenshot Screenshot from 2023-10-05 16-47-51

Ok, nice, thanks !

pom.xml Outdated Show resolved Hide resolved
@bguerin bguerin enabled auto-merge (squash) October 5, 2023 21:50
@bguerin bguerin merged commit 4951812 into jenkinsci:master Oct 5, 2023
15 checks passed
@olamy olamy deleted the spotless branch October 9, 2023 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants