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

refactor/versioning #12

Merged
merged 9 commits into from
Jul 25, 2018
Merged

refactor/versioning #12

merged 9 commits into from
Jul 25, 2018

Conversation

BrunnerLivio
Copy link
Contributor

@BrunnerLivio BrunnerLivio commented Jul 9, 2018

  • Update Readme
  • Update docker image
  • Update add-version.sh script for timestamp folder generating
  • Rename older versions
  • Update NPM package
  • Add Add hydrocannon #10 version
  • Update test cases

Fix for issue #11

@BrunnerLivio BrunnerLivio changed the title refactor/versioning [DO-NOT-MERGE] refactor/versioning Jul 9, 2018

docker run -it --rm --name my-maven-project -v "$HOME/.m2":/root/.m2 -v `pwd`:/usr/src/mymaven -w /usr/src/mymaven maven:3.2-jdk-8 mvn clean package exec:java -Dexec.mainClass="com.pokebattler.gamemaster.GenerateJSON" -Dexec.args="${PROTOBUF_FILE} versions/${VERSION_TO_ADD}/GAME_MASTER.json"
java -Dexec.mainClass="com.pokebattler.gamemaster.GenerateJSON" -Dexec.args="${PROTOBUF_FILE} versions/${VERSION_TO_ADD}/GAME_MASTER.json"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Getting the error

Usage: java [-options] class [args...]
           (to execute a class)
   or  java [-options] -jar jarfile [args...]
           (to execute a jar file)
...

@celandro you know how I the command should be called correctly to generate the GAME_MASTER.json?


docker run -it --rm --name my-maven-project -v "$HOME/.m2":/root/.m2 -v `pwd`:/usr/src/mymaven -w /usr/src/mymaven maven:3.2-jdk-8 mvn clean package exec:java -Dexec.mainClass="com.pokebattler.gamemaster.GenerateJSON" -Dexec.args="${PROTOBUF_FILE} versions/${VERSION_TO_ADD}/GAME_MASTER.json"
java -cp target/pokemongo-game-master-2.28.1.jar com.pokebattler.gamemaster.GenerateJSON "${PROTOBUF_FILE} versions/${VERSION_TO_ADD}/GAME_MASTER.json"
Copy link
Contributor Author

@BrunnerLivio BrunnerLivio Jul 9, 2018

Choose a reason for hiding this comment

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

I get the following error:

Error: Could not find or load main class com.pokebattler.gamemaster.GenerateJSON

Anyone knows a fix? @celandro

by calling

docker build -t "$USER/pokemongo-game-master" .
docker run \
  -v versions:/var/lib/pokemongo-game-master/versions \
  -it "$USER/pokemongo-game-master" \
  -f ./versions/0.57.1/GAME_MASTER.protobuf -v "0.69.9"

@celandro
Copy link
Collaborator

celandro commented Jul 9, 2018 via email

@BrunnerLivio
Copy link
Contributor Author

@celandro I've fixed it, but it is not really efficient at the moment. I'm currently calling mvn clean package inside the bin/add-version.sh file. The build process should be inside the Dockerfile, but somehow I could not get it running because of the exec: part. Not really a java expert, maybe you know the issue.

@BrunnerLivio
Copy link
Contributor Author

I've changed the add-version.sh script so it will generate the current timestamp and set it as version number.

For example, when calling the docker run script, it will create a new folder with the current timestamp e.g. 20180710083743 (= "+%Y%m%d%H%M%S"). You can change the timestamp by setting the -t parameter. If --latest is set, it will automatically update the latest-version.txt and copy the files to the latest folder.

@BrunnerLivio BrunnerLivio force-pushed the refactor/version-numbers branch from 0b3fe61 to 3476932 Compare July 10, 2018 10:50
@BrunnerLivio BrunnerLivio changed the title [DO-NOT-MERGE] refactor/versioning refactor/versioning Jul 10, 2018
@BrunnerLivio
Copy link
Contributor Author

Can someone review this PR? @celandro @dandesousa

Copy link
Collaborator

@celandro celandro left a comment

Choose a reason for hiding this comment

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

I think getting the timestamp out of the file is pretty important so it will be consistent if run on any machine with the same proto file

done

if [ "${PROTOBUF_FILE}" == "" ]; then
print_error_and_exit "Missing protobuf file"
fi

if [ "${VERSION_TO_ADD}" == "" ]; then
print_error_and_exit "Missing version to add"
if [ "${TIMESTAMP}" == "" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

would be better if the timestamp was grepped out of the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh missunderstood this. I thought we should not use the timestamp of the GAME_MASTER file. But it makes more sense this way, will change that

CONTRIBUTING.md Outdated
## Generate GAME_MASTER.json

You can generate a new GAME_MASTER.json using the following command.
It will create a new folder `versions/0.69.9`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is now datestamp based not niantic versions

@BrunnerLivio BrunnerLivio force-pushed the refactor/version-numbers branch from 3476932 to 82c20e0 Compare July 25, 2018 08:33
@BrunnerLivio
Copy link
Contributor Author

Can you review it again @celandro ?

Copy link
Collaborator

@celandro celandro left a comment

Choose a reason for hiding this comment

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

LGTM

-v "$HOME/.gitconfig:/root/.gitconfig" \
-it "$USER/pokemongo-game-master" \
-f /var/lib/pokemongo-game-master/GAME_MASTER.protobuf --latest

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will add a windows version of this command when I have time. The syntax for current and home directory is a little different

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be nice if you can do that. Do not want to bother around with windows if I don't have to :)

@BrunnerLivio BrunnerLivio merged commit 5200096 into master Jul 25, 2018
@Furtif Furtif deleted the refactor/version-numbers branch July 27, 2019 18:32
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.

2 participants