-
Notifications
You must be signed in to change notification settings - Fork 27
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
🐛 Fix labels validation print #251
Conversation
I believe the CI uses "previous" kantra image, that was reason it didn't fail on original PR and looks red on this PR. Works with locally built image based on this PR. |
@aufi I don't think CI uses previous image, we build one in one of the steps unless I am missing something. |
I think we need to update the parsing logic to accomodate for weirdness in different shells (thinking about windows too). Can we just match a regex for label value format on every line of output https://github.com/konveyor/analyzer-lsp/blob/82b3f8b673cbfc178259faf6e35846bc0534d409/engine/labels/labels.go#L21 |
From what I can see locally, the
Will look on that to be sure I'm not wrong. |
Sounds good, just question what if a line would have more than one match of the regex? (fail I'd suggest) |
@pranavgaikwad I noticed the same issue as @aufi while I was debugging #247 where the |
@eemcmullan looks like this is always setting the default value? Lines 89 to 96 in 52f87d7
Shouldn't user input values take precedence over defaults? |
@@ -85,8 +85,13 @@ func (c *Config) trySetDefaultPodmanBin(file string) (found bool, err error) { | |||
} | |||
|
|||
func (c *Config) loadRunnerImg() error { | |||
// TODO(maufart): ensure Config struct works/parses it values from ENV and defaults correctly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick friday-afternoon solution, will continue/follow-up on Monday. Seems the Config mechanism is not setup correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is problem affects more config options, submitted an issue #252 expecting it is a follow-up, but changing things unrelated to this PR.
Fixing fmt.Fprintln to correctly recognize first argument as a writer buffer. Bug was introduced in konveyor#243 Should fix kantra CI. Signed-off-by: Marek Aufart <[email protected]>
Signed-off-by: Marek Aufart <[email protected]>
Signed-off-by: Marek Aufart <[email protected]>
This reverts commit 2950638. Signed-off-by: Marek Aufart <[email protected]>
222d5c4
to
af1e533
Compare
Signed-off-by: Marek Aufart <[email protected]>
There was a conflict with package-level variables from https://github.com/konveyor/kantra/blob/main/cmd/version.go#L12-L16 partially overlapping Settings fields names. Updated Settings to load env with tags (default) as the first step and adjusting it in custom methods as the second one (and use variables within the Settings struct, not outsite). |
Signed-off-by: Marek Aufart <[email protected]>
f4b0f4b
to
3d5e578
Compare
Signed-off-by: Marek Aufart <[email protected]>
55c0c5a
to
5de6055
Compare
Fixing fmt.Fprintln to correctly recognize first argument as a writer buffer. Bug was introduced in my PR #243
This should fix kantra CI.