Strongly + 1 to put core changes behind feature flags so that they can be 
rolled out (and removed if necessary )
in a controlled way. 
Tests should be running successfully for having the feature flag enabled or 
disabled.
In the near past it took a considerable amount of my time to adapt test cases 
for some of the features
introduced in the last time to make them work in all cases.

Regards,
Martin
 


> On 5. Jul 2019, at 06:59, Chetan Mehrotra <chetan.mehro...@gmail.com> wrote:
> 
> +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 <style9...@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

Reply via email to