openwhisk-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chetan Mehrotra <chetan.mehro...@gmail.com>
Subject Re: Allow temporal unused codes to include a big change.
Date Fri, 05 Jul 2019 04:59:29 GMT
+1 to commit incrementally to master.

If the changes touch the core logic then we can possibly have them
backed by feature flags and have them disabled by default and enabled
for test case. Further it would be preferable that whatever we commit
(at any stage) it should have required test coverage. This would
ensures that the sub parts of work in progress bigger feature are
backed by unit tests.

regards
Chetan Mehrotra

On Thu, Jul 4, 2019 at 9:35 PM Dominic Kim <style9595@gmail.com> wrote:
>
> Hello, whiskers.
>
> I am trying to contribute this:
> https://github.com/apache/incubator-openwhisk/pull/4532
>
> At first, I tried to open a base branch and merge all incremental PRs into
> the branch.
> And finally mege the base branch into the master after the implementation
> is done.
> Surely, reviewers would review all the subsequent PRs and it will be based
> on SPI and disabled by default at first, there would be no big issue I
> think.
>
> And Markus suggested to incrementally merge all PRs into the master instead
> of having a big base branch.
> https://github.com/apache/incubator-openwhisk/pull/4532#issuecomment-508519119
>
> With the former, we don't need to include temporal unused codes but need to
> include a big possibly disruptive PR at once.
> With the latter, there will be some temporal unused components in the
> master at some point, but we can make sure merged codes do not induce any
> issue. And actually the latter is easier for me as well : )
>
> If we all agree with including the temporal unused codes in the master, I
> am happy to work in the way Markus suggested.
>
> One example of a temporal code is this:
> https://github.com/apache/incubator-openwhisk/pull/4532/files#diff-1d7110b32c507a6ef4ac956c287e77ebR24
>
> Since there is no other component, the "scheduler" cannot initialize all
> components in the main function and there is only `println("Hello")` in the
> initial version of the main function.
>
>
> Please let me know your thoughts.
>
> Best regards
> Dominic

Mime
View raw message