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 #11726: Automate creation of settings.xml file for release #11743

Merged
merged 1 commit into from Jun 26, 2022
Merged

Issue #11726: Automate creation of settings.xml file for release #11743

merged 1 commit into from Jun 26, 2022

Conversation

Rahulkhinchi03
Copy link
Member

@Rahulkhinchi03 Rahulkhinchi03 commented Jun 14, 2022

Resolves: Issue #11726

  • Create release-settings.xml file.
  • Create prepare-setting.sh and it copy release-setting.xml to .ci-temp
  • Put values of environmental variables in generated file in .ci-temp
  • Make backup of original file to ~/.m2/settigns.xml.backup.{date}
  • copy generated file to ~/.m2/settigns.xml

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/settings.sh Outdated Show resolved Hide resolved
.ci/settings.sh Outdated Show resolved Hide resolved
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/prepare-settings.sh Outdated Show resolved Hide resolved
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/prepare-settings.sh Outdated Show resolved Hide resolved
.ci/prepare-settings.sh Outdated Show resolved Hide resolved
.ci/prepare-settings.sh Outdated Show resolved Hide resolved
.ci/prepare-settings.sh Outdated Show resolved Hide resolved
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/prepare-settings.sh Show resolved Hide resolved
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/prepare-settings.sh Outdated Show resolved Hide resolved
.ci/prepare-settings.sh Outdated Show resolved Hide resolved
.ci/prepare-settings.sh Outdated Show resolved Hide resolved
.ci/release-settings.xml Outdated Show resolved Hide resolved

if cmp -s "$file1" "$file2"; then
date=$(date +"%m%d%y")
mv ~/.m2/settings.xml ~/.m2/settings.xml.backup."${date}"
Copy link
Member

Choose a reason for hiding this comment

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

indentation

Copy link
Member Author

Choose a reason for hiding this comment

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

Changes Done

.ci/prepare-settings.sh Outdated Show resolved Hide resolved
mv ~/.m2/settings.xml ~/.m2/settings.xml.backup."${date}"
fi

cp .ci-temp/release-settings.xml ~/.m2/settings.xml
Copy link
Member

Choose a reason for hiding this comment

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

you have variables for these files

Copy link
Member Author

Choose a reason for hiding this comment

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

We have variables for these files but we can use this only if we run those in Travis CI.

eg: MVN_SETTINGS=${TRAVIS_HOME}/.m2/settings.xml

Copy link
Member

Choose a reason for hiding this comment

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

we should not use CI specific variables, this script should be executable on any CI and any local.

Copy link
Member

Choose a reason for hiding this comment

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

I think exact path might work better then variable - it is almost unlikely to change

Copy link
Member

Choose a reason for hiding this comment

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

what I mean is that several lines above it is

TEMP_SETTING="./.ci-temp/release-settings.xml"
SETTING="~/.m2/settings.xml"

so this command can be just cp $TEMP_SETTING $SETTING and duplcation is avoided

Copy link
Member

Choose a reason for hiding this comment

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

now I see it, thanks a lot.
done.

.ci/prepare-settings.sh Outdated Show resolved Hide resolved
Copy link
Member

@strkkk strkkk left a comment

Choose a reason for hiding this comment

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

see comments above

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/release-settings.xml Outdated Show resolved Hide resolved
.ci/release-settings.xml Outdated Show resolved Hide resolved
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/prepare-settings.sh Outdated Show resolved Hide resolved
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/prepare-settings.sh Outdated Show resolved Hide resolved
.ci/prepare-settings.sh Outdated Show resolved Hide resolved
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 will merge this as soon as CI pass.

@strkkk , if you have more items, please keep raising them, we will fix them in next PRs

@romani romani merged commit c666b0e into checkstyle:master Jun 26, 2022
replace SONATYPE_USER
replace SONATYPE_PWD
replace GPG_PASSPHRASE
replace GPG_KEY
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these names be more Checkstyle specific? IE: CS_GPG_KEY or CHECKSTYLE_GPG_KEY

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure for now to put CS as prefix for all.

Copy link
Member

Choose a reason for hiding this comment

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

Aren't these just environment variables the user has set on their machine going by checkForVariable? Are these environment variables needed some place else besides for this script?

Copy link
Member

Choose a reason for hiding this comment

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

Just a variables, not used in other scripts.

We never named variables with CS prefix, I not sure that we should start now.

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.

None yet

4 participants