Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

improved version of "fixVersions.sh" #84

Merged
merged 4 commits into from
Dec 20, 2018

Conversation

pngowda
Copy link
Contributor

@pngowda pngowda commented Oct 10, 2018

@kthoms @cdietrich
Hi
Here is the improved version of "fixVersionsScript.sh" .
could you please review this pull request. and let me know if some changes required.
Regards,
Prajwal

@cdietrich cdietrich requested a review from kthoms October 10, 2018 06:28
@cdietrich cdietrich added this to the Release_2.16 milestone Oct 10, 2018
@cdietrich
Copy link
Member

@pngowda can you please signoff the commit?

@kthoms
Copy link

kthoms commented Oct 10, 2018

For me the script takes long (>1min) when executed:

./fixVersions.sh -f 2.15.0
# from_version: 2.15.0
# to_version:
# 'to_version' is unset or set to the empty string
# deriving 'to_version' from 'from_verison'
# to_version: 2.16.0
# OS: Mac OS

It creates files with double quote at the end:

screenshot 94

fixVersions.sh Outdated Show resolved Hide resolved
fixVersions.sh Outdated Show resolved Hide resolved
@pngowda pngowda closed this Oct 10, 2018
@kthoms
Copy link

kthoms commented Oct 10, 2018

Why was the PR closed?

@pngowda
Copy link
Contributor Author

pngowda commented Oct 10, 2018

@kthoms @tivervac
could you please review.
have done some fixes as per the review comment. I use linux as my native OS, in that it the run time is not more than 2 sec. but in Mac its taking more time I have no clue.

@cdietrich
Copy link
Member

hmm iplog bot is in a bad state again :(

@cdietrich
Copy link
Member

Copy link
Member

@cdietrich cdietrich left a comment

Choose a reason for hiding this comment

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

iplog (signoff email seems wrong)
we need to run the script on all repos.
have a look how other scripts use "allDirectories"

@tivervac
Copy link
Contributor

@pngowda LGTM

Copy link

@kthoms kthoms left a comment

Choose a reason for hiding this comment

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

Can't specify Milestone/Release candidates
Target version would be for example 2.16.0.M1 or 2.16.0.RC1

@kthoms
Copy link

kthoms commented Oct 12, 2018

Also I'd expect that a development version would be specified with -SNAPSHOT suffix. How to distinguish between the dev and release version otherwise? For certain pom.xml the -SNAPSHOT must be removed for the final release, for others (used for Tycho builds) it has to stay.

@kthoms
Copy link

kthoms commented Oct 12, 2018

That said, I expect that it should be valid to execute with fixVersions.sh -f 2.16.0-SNAPSHOT -t 2.16.0.M1 or fixVersions.sh -f 2.16.0.RC1 -t 2.16.0

@cdietrich
Copy link
Member

I would expect two scripts for that. I don’t think that can be done with one

@cdietrich
Copy link
Member

cdietrich commented Oct 12, 2018

I created the initial version exactly for the snapshot usecase Nothing else. Prajwal added the ask for versions you wanted to have

@kthoms
Copy link

kthoms commented Oct 12, 2018

The script only works in the umbrella repo. When I execute it in the directory of xtext-core, nothing happens. I miss a bit feedback which files are actually processed.

@cdietrich
Copy link
Member

@kthoms see #84 (review)

@pngowda
Copy link
Contributor Author

pngowda commented Oct 12, 2018

@kthoms @cdietrich

I discussed with Christian some time ago and I got to know that, there are different cases here.

snapshot to snapshot
milestone to snapshot
milestone to relase
release to snapshot
etc...

right now the script handles snapshot to snapshot(initial version from Christian) so, we need to finalize on these cases and define the set of files to be changed accordingly. then it would be easy for me to implement the script as generic as possible.

@szarnekow
Copy link
Contributor

Is there still progress being made on this PR?

@pngowda
Copy link
Contributor Author

pngowda commented Dec 7, 2018

right now it handles Snapshot to Snapshot bumping.need to define set of files to be changed for other cases.

@cdietrich
Copy link
Member

@kthoms please review

@kthoms
Copy link

kthoms commented Dec 11, 2018

Did not work (macOS)

./fixVersions.sh -f 2.16.0
# from_version: 2.16.0
# to_version:
# bump_to:
# 'to_version' is unset or set to the empty string
# deriving 'to_version' from 'from_verison'
# to_version: 2.17.0
./fixVersions.sh: line 121: [: ==: unary operator expected

@kthoms
Copy link

kthoms commented Dec 11, 2018

Please do not forget to squash & sign commit.

fixVersions.sh Outdated Show resolved Hide resolved
fixVersions.sh Outdated Show resolved Hide resolved
Prajwal added 3 commits December 14, 2018 14:37
Signed-off-by: pgowda <prajwal.gowda@itemis.de>
Signed-off-by: Prajwal Gowda <pgowda@itemis.de>
Signed-off-by: Prajwal Gowda <pgowda@itemis.de>
Signed-off-by: Prajwal Gowda <pgowda@itemis.de>
@cdietrich
Copy link
Member

@kthoms please reexamine

@cdietrich
Copy link
Member

@kthoms what is the state of the review?

Copy link

@kthoms kthoms left a comment

Choose a reason for hiding this comment

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

Failure on macOS:

Processing .../xtext-dev/git/xtext-maven
find: illegal option -- t
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
       find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]

@kthoms
Copy link

kthoms commented Dec 20, 2018

Problem was usage of the currentDirectory property. This can be avoided when switching to the repository directory and then execute find on the current directory .. Makes the code also easier.

Copy link

@kthoms kthoms left a comment

Choose a reason for hiding this comment

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

Thanks @pngowda

@kthoms kthoms merged commit c729e37 into eclipse:master Dec 20, 2018
@kthoms kthoms added the releng label Dec 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants