nimbella / aio-cli-plugin-runtime

Nimbella Fork of the Adobe I/O Runtime plugin for Apache OpenWhisk.
https://nimbella.com
Apache License 2.0
2 stars 1 forks source link

activation logs --limit enhancements #23

Closed rabbah closed 3 years ago

rabbah commented 3 years ago

Is your feature request related to a problem? Please describe.

rabbah commented 3 years ago

The best place to patch this is in the aio-lib-runtime repo. I've opened an issue https://github.com/adobe/aio-lib-runtime/issues/27 and have an implementation on this fork https://github.com/adobe/aio-lib-runtime/compare/master...rabbah:log-banner?expand=1.

rabbah commented 3 years ago

@joshuaauerbachwatson are we still dependent on a lower version of chalk (v2.4.2 vs latest which is v4.0)? (The interface to using chalk.bold/dim is different, the former uses a format string and the latter does not from a cursory reading of the apis.)

In the linked branch, I used the latest version of chalk. Also, if the patch isn't accepted upstream, is there a convenient way to apply the patch in our own build without having to also fork the aio-lib-runtime repo?

joshuaauerbachwatson commented 3 years ago

@joshuaauerbachwatson are we still dependent on a lower version of chalk

We are not.

is there a convenient way to apply the patch in our own build without having to also fork the aio-lib-runtime repo

Yes, using patch-package. There are numerous patches against aio-lib-runtime in the cloud-workbench build (see cloud-workbench/patches). Those particular patches are aimed at avoiding webpack / workbench initialization problems. We so far have none that we apply at CLI level. But, the nim build is set up to use patch-package, and we could easily be patching aio-lib-runtime there. You could also use patch-package in the aio repo. Adobe used to use it and abandoned it. I don't know why. Given the way we package things the only advantage to doing it in the aio repo is independent testing. But, if this is something that has been rejected upstream we could decide that testing it as part of nim instead of as part of aio is adequate.

rabbah commented 3 years ago

Thanks for the explanation. Will see how the issue is received upstream.

Re chalk, should this commit be dropped then https://github.com/nimbella/aio-cli-plugin-runtime/commit/490312d498ea81c0cc6fa0fb6c286ae9cfc30e06 (at least the chalk downgrade).

joshuaauerbachwatson commented 3 years ago

Re chalk, should this commit be dropped then 490312d

I believe all chalk workarounds can be dropped. The reason for the problem was a glitch in dependency handling in oclif which has since been fixed.

rabbah commented 3 years ago

The upstream change is now merged. The solutions evolved through feedback on the PR so I will adapt our cli fork accordingly. There is a question of how to pick up the change in aio-lib-runtime which I'll take up in Slack for now.

rabbah commented 3 years ago

A new release of aio-lib-runtime is out (v1.3.1) which includes the required change. I have updated our dev branch to bump the package version. Once this is picked up by nimbella-cli @joshuaauerbachwatson or I can close this issue.

joshuaauerbachwatson commented 3 years ago

I have updated our dev branch to bump the package version.

Available evidence does not support this assertion. The current tag (v2021-02-03-1) still identifies the commit at the head of the dev branch (6c8b6a21) and, at that commit, the version of aio is still 3.0.0.

Perhaps not all your changes were pushed or were pushed to your fork rather than "our" fork in the nimbella org?

rabbah commented 3 years ago

🤦 i pushed to my fork only; now fixed. Shall I apply a new tag?

joshuaauerbachwatson commented 3 years ago

Shall I apply a new tag?

I can take it from here.

rabbah commented 3 years ago

Hold off - sorry about that. Travis didn't approve (coverage dropped so allow me a chance to track it down - 100% LOC tested but global is 99.94%).

joshuaauerbachwatson commented 3 years ago

Ok. Nothing pushed yet. I'll back out the adoption. I was going to put it on a branch anyway because I anticipate there could be problems with the workbench build which I don't want to try to fix in real time.

joshuaauerbachwatson commented 3 years ago

Rodric fixed things and re-pushed. I did the adoption on (local) branches. There was no problem with the workbench build.

I am proceeding to merge the adoption.

joshuaauerbachwatson commented 3 years ago

The adoption is merged. @rabbah I don't know what follow-up testing you want to do. Since this is in the aio repo I will leave the closing timing to you.

rabbah commented 3 years ago

Changes are already tested. Thank you for rolling it into our cli.