Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Different approach on merging application definition#20054
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ro0NL commentedSep 28, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
So.. any thoughts on this? I think this is way more obvious than the current approach. The list command not giving the application-wide options is a bug.. right? However they were more or less explicitly left out... any opinion on this? edit: yeah.. after#20090 and rebase 2.7 imo. |
ro0NL commentedDec 29, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Not sure about intentions here. It's done rather explicitly. The global options do work though with |
l-vo commentedNov 29, 2018
Tested the changes on 3.4 and this approach seems to be working properly. |
Uh oh!
There was an error while loading.Please reload this page.
ro0NL commentedNov 30, 2018
status: needs work |
fabpot commentedAug 11, 2020
@ro0NL I'm digging the old PRs right now and stumbled on this one. I see that you marked it as "Needs work", is it still relevant? |
ro0NL commentedAug 11, 2020
@fabpot i'd say yes. It boils down to Not sure why i marked it "needs work" 😅 i figure to come to some sort of conslusion here. |
fabpot commentedAug 11, 2020
Ok, can you rebase on current master? I will test it on a real application to see how it behaves. |
ro0NL commentedAug 11, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@fabpot should be good. |
ro0NL commentedAug 12, 2020
@fabpot console component passes tests 👍 |
fabpot left a comment
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.
Great job!
fabpot commentedAug 13, 2020
Thank you@ro0NL. |
Uh oh!
There was an error while loading.Please reload this page.
Before/After:
This could deprecate
getNativeDefinitionor make it a feature as right now it's internal and unused.edit: resolved the BC break.
edit2: question is.. should this target 2.7?