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

Issue #943: 'Bump CS version in REPO' is not working #954

Merged
merged 2 commits into from Dec 7, 2022

Conversation

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

I need to look deeper.
Version bump should not try to compile project.

We just need update digits in file and let other CI job do all compilation in proper form.

Any idea why we need compilation for update of number in file?

We can use sed instead if there is something crazy.

We use mvn plugin for version bump only because it is simple not hacky command.
But all we need is updated digits in xml file.

https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/pom-version-bump.sh

@rnveach
Copy link
Contributor

rnveach commented Dec 4, 2022

@romani see #943 (comment) . That is the reason.

Is there any chance of asking eclipsecs to be in maven?

@romani
Copy link
Member

romani commented Dec 4, 2022

Even dependabot service never compile target project. It simply update numbers in text file. We need same.

@rnveach
Copy link
Contributor

rnveach commented Dec 4, 2022

Even dependabot service never compile target project.

I don't believe we are compiling, but doing version verification. I am not sure, this is all eclipse's process kicking off here.

dependatbot is not enabled for this POM because of this reason. I do not know if dependabot would have the same issues.

Either way, if you want to not do this then we will have to rely on manually parsing.

@stoyanK7
Copy link
Contributor Author

stoyanK7 commented Dec 5, 2022

We can use xmlstarlet and do this. Though it removes empty lines as well.

xmlstarlet edit --inplace --update "/_:project/_:properties/_:checkstyle.version" --value "1.1.1" eclipsecs-sevntu-plugin/pom.xml
diff --git a/eclipsecs-sevntu-plugin/pom.xml b/eclipsecs-sevntu-plugin/pom.xml
index b1a3911a..90d2ad72 100644
--- a/eclipsecs-sevntu-plugin/pom.xml
+++ b/eclipsecs-sevntu-plugin/pom.xml
@@ -1,40 +1,32 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
-
   <modelVersion>4.0.0</modelVersion>
-
   <!-- we use eclipse-cs parent to ease our compilation and always be up-to-date -->
-
   <parent>
     <relativePath>../eclipse-pom.xml</relativePath>
     <groupId>com.github.sevntu-checkstyle</groupId>
     <artifactId>parent</artifactId>
     <version>1.44.0</version>
   </parent>
-
   <groupId>com.github.sevntu-checkstyle</groupId>
   <artifactId>eclipsecs-sevntu-plugin</artifactId>
   <packaging>eclipse-plugin</packaging>
-
   <name>eclipse-cs Sevntu Checkstyle Plugin</name>
-
   <licenses>
     <license>
       <name>LGPL-2.1+</name>
       <url>http://www.gnu.org/licenses/old-licenses/lgpl-2.1.txt</url>
     </license>
   </licenses>
-
   <properties>
     <eclipsecs.version>10.4.0-SNAPSHOT</eclipsecs.version>
     <!-- verify time version -->
-    <checkstyle.version>10.4</checkstyle.version>
+    <checkstyle.version>1.1.1</checkstyle.version>
     <checkstyle.configLocation>https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-${checkstyle.version}/config/checkstyle_c

Or sed

sed -i -e '/<properties>/,/<\/properties>/ s|<checkstyle.version>.*</checkstyle.version>|<checkstyle.version>1.1.1</checkstyle.version>|g' eclipsecs-sevntu-plugin/pom.xml 
diff --git a/eclipsecs-sevntu-plugin/pom.xml b/eclipsecs-sevntu-plugin/pom.xml
index b1a3911a..ede68465 100644
--- a/eclipsecs-sevntu-plugin/pom.xml
+++ b/eclipsecs-sevntu-plugin/pom.xml
@@ -28,7 +28,7 @@
   <properties>
     <eclipsecs.version>10.4.0-SNAPSHOT</eclipsecs.version>
     <!-- verify time version -->
-    <checkstyle.version>10.4</checkstyle.version>
+    <checkstyle.version>1.1.1</checkstyle.version>
     <checkstyle.configLocation>https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-${checkstyle.version}/config/checkstyle_checks.xml</checkstyle.configLocation>
     <checkstyle.header>https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/java.header</checkstyle.header>
     <checkstyle.regexp.header>https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/java_regexp.header</checkstyle.regexp.header>

@romani
Copy link
Member

romani commented Dec 5, 2022

Please use sed

@coveralls
Copy link

coveralls commented Dec 5, 2022

Coverage Status

Coverage remained the same at 98.798% when pulling 14974c7 on stoyanK7:issue/943 into 6c68f16 on sevntu-checkstyle:master.

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

Items

.ci/bump-cs-version-in-esclipsecs-plugin.sh Outdated Show resolved Hide resolved
@@ -10,6 +10,6 @@ fi
VERSION=$1
cd eclipsecs-sevntu-plugin

mvn -e --no-transfer-progress versions:set-property -DgenerateBackupPoms=false -Dproperty=checkstyle.version -DnewVersion="$VERSION"
sed -i -e "/<properties>/,/<\/properties>/ s|<checkstyle.version>.*</checkstyle.version>|<checkstyle.version>$VERSION</checkstyle.version>|g" pom.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

My only question is can we split this line into 2 since it is 150 characters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

✔ ~/Open Source/sevntu.checkstyle [issue/943|✔] 
19:51 $ git diff
✔ ~/Open Source/sevntu.checkstyle [issue/943|✔] 
19:51 $ ./.ci/bump-cs-version-in-eclipsecs-plugin.sh 10.5.1
Version updated to 10.5.1 at eclipsecs-sevntu-plugin/pom.xml
✔ ~/Open Source/sevntu.checkstyle [issue/943|✚ 1] 
19:51 $ git diff
diff --git a/eclipsecs-sevntu-plugin/pom.xml b/eclipsecs-sevntu-plugin/pom.xml
index b1a3911a..68d337e0 100644
--- a/eclipsecs-sevntu-plugin/pom.xml
+++ b/eclipsecs-sevntu-plugin/pom.xml
@@ -28,7 +28,7 @@
   <properties>
     <eclipsecs.version>10.4.0-SNAPSHOT</eclipsecs.version>
     <!-- verify time version -->
-    <checkstyle.version>10.4</checkstyle.version>
+    <checkstyle.version>10.5.1</checkstyle.version>
     <checkstyle.configLocation>https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-${checkstyle.version}/config/checkstyle_checks.xml</checkstyle.configLocation>
     <checkstyle.header>https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/java.header</checkstyle.header>
     <checkstyle.regexp.header>https://raw.githubusercontent.com/checkstyle/checkstyle/master/config/java_regexp.header</checkstyle.regexp.header>

@stoyanK7
Copy link
Contributor Author

stoyanK7 commented Dec 6, 2022

Fixed small typo in filename that managed to slip away a few days ago,

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

Ok to merge

@rnveach rnveach merged commit 282344a into sevntu-checkstyle:master Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'Bump CS version in REPO' is not working
4 participants