Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

stdlibConfig strictMode should not fail if there are existing flags #89

Merged
merged 3 commits into from
Jul 9, 2021

Conversation

EngHabu
Copy link
Contributor

@EngHabu EngHabu commented Jul 6, 2021

TL;DR

Update strictMode to capture existing flags in a flagset before parsing params. This way it can avoid failing when there are flags defined outside of stdlibConfig world.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes flyteorg/flyte#1209

Signed-off-by: Haytham Abuelfutuh <[email protected]>
@codecov
Copy link

codecov bot commented Jul 6, 2021

Codecov Report

Merging #89 (b15a9b7) into master (73eeac3) will decrease coverage by 0.30%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
- Coverage   65.49%   65.19%   -0.31%     
==========================================
  Files          59       59              
  Lines        2275     2284       +9     
==========================================
- Hits         1490     1489       -1     
- Misses        642      652      +10     
  Partials      143      143              
Flag Coverage Δ
unittests 65.19% <0.00%> (-0.31%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
config/viper/viper.go 7.65% <0.00%> (-0.37%) ⬇️
sets/generic_set.go 93.33% <0.00%> (-1.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73eeac3...b15a9b7. Read the comment docs.

Signed-off-by: Haytham Abuelfutuh <[email protected]>
@EngHabu EngHabu marked this pull request as ready for review July 6, 2021 20:30
@EngHabu EngHabu requested review from pmahindrakar-oss and removed request for katrogan and kumare3 July 6, 2021 20:31
Signed-off-by: Haytham Abuelfutuh <[email protected]>
@EngHabu EngHabu merged commit f64c747 into master Jul 9, 2021
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
)

* Do not error on existing flags

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Add unit test

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Update unit test

Signed-off-by: Haytham Abuelfutuh <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core Feature] stdlibConfig strictMode is too strict
2 participants