indigo-dc / orchestrator

The INDIGO PaaS Orchestrator
https://www.indigo-datacloud.eu/paas-orchestrator
Apache License 2.0
17 stars 21 forks source link

Vault integration #331

Closed mperniola closed 5 years ago

mperniola commented 5 years ago

Full integration of Vault with Marathon added entries in application.properties upgraded spring framework to version 1.5.21

t6pc-bot commented 5 years ago

Can one of the admins verify this patch?

alberto-brigandi commented 5 years ago

add to whitelist

mperniola commented 5 years ago

The copyright was automatically updated by the mycila plugin as it was run in 'format' mode in order to resolve its cryptic error message (header absent) with respect to the new files that were added; in any case, the change made was 2018 in 2019. As for the other formatting problems (?) At the most it was added some "suppress warning" in files that had been opened for consultation or reference and that reported such problems. All other changes are functional Greetings MP

Il 01.08.2019 17:25 Alberto Brigandì ha scritto:

@ALBERTO-BRIGANDI requested changes on this pull request.

Hi @mperniola [1], A lot of files have been changed for reasons out of scope of this PR. In order to properly understand what has been changed, I need from you to modify this PR in order to keep only the changes needed:

  • There's no need to modify the copyright header of files that have not been changed
  • Do not change the formatting of the code that has not been functionally touched by this PR

Please also resolve the issue that is preventing the Travis-ci pipeline to complete successfully

-- You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub [2], or mute the thread [3].

Links:

[1] https://github.com/mperniola [2] https://github.com/indigo-dc/orchestrator/pull/331?email_source=notifications&email_token=AEYH5QJ6ROPVNFZS2TFBJETQCL55HA5CNFSM4IFDZD4KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCAJUQ4A#pullrequestreview-269699184 [3] https://github.com/notifications/unsubscribe-auth/AEYH5QKAVRCM7J4ME43K46TQCL55HANCNFSM4IFDZD4A

-- Michele Perniola, M.S.C.S. INFN (National Institute for Nuclear Physics) - Bari Via Orabona, 4 70125 – Bari email: michele.perniola@ba.infn.it skype: michele.perniola

alberto-brigandi commented 5 years ago

The copyright was automatically updated by the mycila plugin as it was run in 'format' mode in order to resolve its cryptic error message (header absent) with respect to the new files that were added; in any case, the change made was 2018 in 2019.

The mycilia plugin validates and updates the copyright header only if the file has been changed. If you don't change a file, you don't need to update its copyright; if that wasn't true all the CI pipelines on jenkins and travis would fail on validation phase. Therefore, if the plugin is automatically generating this change, there must be some configuration problem in your workspace (the first thing that comes to my mind is git changing automatically the endline character from CR to CRLF or vice-versa, but it could be something else). Anyway, those changes are not needed and make the review task unnecessarily hard to execute, so please remove them.

As for the other formatting problems (?) At the most it was added some "suppress warning" in files that had been opened for consultation or reference and that reported such problems.

I didn't say that there are formatting problems; I said that there are formatting changes (e.g. indentation changes) that are not needed; as I said before, please remove it. Additionally, do not add suppressWarnings if not needed; if there is a warning, 99% of the time there's something that can be corrected or improved; if you add a warning suppression, that will go out of sight

Please, allow me to easily understand what are the functional changes that this PR wants to apply (thus please do not change code if it is not strictly needed)