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