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

Versions updated & reorganization #143

Closed
wants to merge 17 commits into from
Closed

Versions updated & reorganization #143

wants to merge 17 commits into from

Conversation

iCesofT
Copy link
Contributor

@iCesofT iCesofT commented Jun 6, 2020

  • Spring boot to 2.2.7.RELEASE
  • Reorganization (dependencyManagement vs dependencies)

- Spring boot to 2.2.7.RELEASE
- Reorganization (dependencyManagement vs dependencies)
- We allow to use a DB access without admin privileges from the application (using `datasource.flyway.load=false` or `DATASOURCE_FLYWAY_LOAD` environment variable set-up to false).
  We create a Flyway @bean that do not do anything (it doesn't migrate).
- We propose an application.yaml inside Docker with environment variables (see `dpppt-backend-sdk-ws/src/main/docker/application.yaml`).
@martinalig
Copy link
Contributor

martinalig commented Jun 10, 2020

Hi
Thanks for the inputs.
One thing: We do not plan to replace logback with log4j2. Please adapt your pullrequest.

@iCesofT
Copy link
Contributor Author

iCesofT commented Jun 15, 2020

I already changed my code to use logback (using spring-boot-starter-logging).

@@ -1,30 +1,13 @@
<!-- Issue templates and Contributing guides are taken from the fastlane repository and adjusted for our needs (https://github.com/fastlane/fastlane/tree/master/.github/ISSUE_TEMPLATE) and -->

Choose a reason for hiding this comment

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

The and at the end of the sentence looks like a stray/artifact. Or got something lost while copy-editing?

Suggested change
<!-- Issue templates and Contributing guides are taken from the fastlane repository and adjusted for our needs (https://github.com/fastlane/fastlane/tree/master/.github/ISSUE_TEMPLATE) and -->
<!-- Issue templates and Contributing guides are taken from the fastlane repository (https://github.com/fastlane/fastlane/tree/master/.github/ISSUE_TEMPLATE) and adjusted for our needs -->

From a readability perspective, I think would be better to place the URL directly to where the repository is mentioned - but that is personal preference.

/*
* Created by Ubique Innovation AG
* https://www.ubique.ch
* Copyright (c) 2020. All rights reserved.

Choose a reason for hiding this comment

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

Nitpick//Observation/Question: All rights reserved. Is this an outdated copyright headers?
#75 looks like it replaced the All rights reserved. with MPL-2.0
https://github.com/DP-3T/dp3t-sdk-backend/pull/75/files
(This applies to other files of this PR, but I only make one comment - maybe there is a good reason for this header)

…/backend/sdk/ws/security/KeyVault.java


You are right. It is a mistake.

Co-authored-by: Michael Frey <michael.frey@gmx.ch>
@ineiti
Copy link
Collaborator

ineiti commented Jul 29, 2020

Hi - I'm an external contributor, so my comments do not reflect Ubique's team...

  • the PR should be rebased against the latest develop branch
  • I see a lot of changes in the documentation and different pieces of code - is this because it's not rebased?
  • if there are still a lot of changes even after the rebase, I would propose to:
    • explain what you want to do by opening an issue, eventually more than one
    • create one PR per issue

@iCesofT
Copy link
Contributor Author

iCesofT commented Sep 25, 2020

I will create a new PR.

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