Use lowercase APIMapping #346
Conversation
|
/hold for scruntiny |
| return err | ||
| } | ||
|
|
||
| cmd.Println(string(out)) |
There was a problem hiding this comment.
Why this changed?
There was a problem hiding this comment.
It was adding a newline where one already existed.
|
Let me create a Jira ticket to add a unit test for 'generate'. |
|
I'm hesitant to change the mapping file format, because I feel like it did work at some point? I remember manually "hacking" in some simple fake kind at some point, and I think Matt did something similar... The lack of a test makes it hard to tell if/when things broke. But this is still a small piece of preview feature. If changing it brings other benefits, we should do it. What does this give us over the current fix? |
Just tried it out. It seems if lowercase keys are used, that works before the fix. So it's an inflection point. Do we go with what |
dhaiducek
force-pushed
the
lowercase-api
branch
2 times, most recently
from
aec5405 to
2f4b04a
Compare
|
Should we add unit test with capital? |
There is one--your test is unchanged 🙂 |
Signed-off-by: Dale Haiducek <[email protected]>
dhaiducek
force-pushed
the
lowercase-api
branch
from
2f4b04a to
eb9714f
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhaiducek, JustinKuli, yiraeChristineKim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold |
openshift-merge-bot
bot
merged commit 0d84aee
into
open-cluster-management-io:main
Proposed followup to: