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

ocis already does the cli flag parsing #1705

Merged
merged 3 commits into from
Jul 7, 2021
Merged

ocis already does the cli flag parsing #1705

merged 3 commits into from
Jul 7, 2021

Conversation

butonic
Copy link
Member

@butonic butonic commented Feb 23, 2021

The micro service.Init() function applies the options and then parses cli flags, which resets some of the defaults in micro, interfering and overriding our options.

  • remove all service.Init()
  • smoke test in tho browser: login, up & downloud still works
  • test setting a different micro registry / log level
  • test setting micro cli env vars.
  • wrap service.Init() in our http and grpc services so it cannot be called accidentially.
  • check if micros server.Init(), client.Init(), registry.Init() interfere in the same way ...
  • maybe create ocis versions of service, client, registry...?

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@update-docs
Copy link

update-docs bot commented Feb 23, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@butonic
Copy link
Member Author

butonic commented Jul 1, 2021

@refs help! ;-)

@refs
Copy link
Member

refs commented Jul 2, 2021

I individually tested a service (proxy) using cli flags and env variables and it behaves the same way. I'd let this go through CI, add a changelog and merge it.

image

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 2, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@refs refs marked this pull request as ready for review July 2, 2021 12:28
@refs refs requested a review from LukasHirt as a code owner July 2, 2021 12:28
@butonic butonic requested a review from C0rby July 7, 2021 08:34
Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

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

👍

@butonic butonic merged commit cd4b391 into master Jul 7, 2021
@delete-merged-branch delete-merged-branch bot deleted the no-additional-init branch July 7, 2021 08:36
ownclouders pushed a commit that referenced this pull request Jul 7, 2021
Merge: 54e1796 25ca76e
Author: Jörn Friedrich Dreyer <jfd@butonic.de>
Date:   Wed Jul 7 10:36:04 2021 +0200

    Merge pull request #1705 from owncloud/no-additional-init

    ocis already does the cli flag parsing
@micbar micbar mentioned this pull request Jul 13, 2021
21 tasks
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.

3 participants