search

microservices-demo / carts

Carts service for microservices-demo application
https://github.com/microservices-demo/microservices-demo
Apache License 2.0
41 stars 1.98k forks source link

Bump to Spring Boot 2 #36

Closed mjlodge closed 4 years ago

mjlodge commented 4 years ago

This change bumps up the Spring Boot version to 2. There's a change to the Spring Data API so code now uses findById() instead of findOne().

dholbach commented 4 years ago

Hi @mjlodge - thanks for the PR.

I'm not a Java expert - far from it, but wouldn't we want to change these instances too?

[daniel@reef carts ]$ git grep -i "jdk.*8"
Dockerfile:FROM java:openjdk-8-alpine
scripts/build.sh:$DOCKER_CMD run --rm -v $HOME/.m2:/root/.m2 -v $CODE_DIR:/usr/src/mymaven -w /usr/src/mymaven maven:3.2-jdk-8 mvn -q -DskipTests package
test/component.py:                   '/usr/src/mymaven', 'maven:3.2-jdk-8', 'mvn', 'integration-test']
test/coveralls.py:                   'maven:3.2-jdk-8',
test/unit.py:                   '/usr/src/mymaven', 'maven:3.2-jdk-8', 'mvn', '-q', 'test']
[daniel@reef carts ]$
richardcase commented 4 years ago

Thanks @mjlodge - LGTM.

It probably is a good idea to update the maven docker image as well to jdk 11 as @dholbach mentions. What do you think?

mjlodge commented 4 years ago

Bumped Maven test container to 3.6/JDK 11.

mjlodge commented 4 years ago

Says merging is still blocked. Someone with write access needs to approve.

dholbach commented 4 years ago

@jpellizzari @philwinder Can one of you merge this?

dholbach commented 4 years ago

Looks like CI is unhappy: https://travis-ci.org/github/microservices-demo/carts/builds/712117698

$ docker login -u $DOCKER_USER -p $DOCKER_PASS;
WARNING! Using --password via the CLI is insecure. Use --password-stdin.
Error response from daemon: Get https://registry-1.docker.io/v2/: unauthorized: incorrect username or password
mjlodge commented 4 years ago

This is something only you can fix -- how do you want to do Docker auth?

The way the script was set up, all non-Weaveworks PRs would fail CI. So I restored the previous script, which has this line in it -- which is just for you (Weaveworks) to push new images to Dockerhub.