[PATCH openbmc-test-automation 00/60] Pull Manjunath's automation repository

[Cyril Bur]

On Mon, 11 Jan 2016 10:49:26 -0600
OpenBMC Patches <openbmc-patches at stwcx.xyz> wrote:

> Pull Manjunath's repository: https://github.com/mkumatag/openbmc-automation
> into openbmc.
> 

Ummmm what happened here? It looks like the entire series is sent twice?

I've had an initial skim read and it looks to me like you can squash A LOT of
those patches together.

The commits don't need to be a story of how you got from no code to the
this point in time. Each commit should add a feature and associated
documentation. It looks like most of patches 0-6 can be one commit, same for
some subsequent patches.

Heres an example of some issues I've noticed:

In patch 2 you add tests/test_ssh.robot but then in patch 7 you delete 
tests/test_ssh.robot. There are several examples of a patch adding a
significant chunk of work and then a later patch removing this chunk of work, if
after the series the chunk of work won't be in the repo then it shouldn't have
appeared in the series at all.

Patch 26 "Fix errors" should be squashed into the broken commit. Patch 22 looks
that way too and Patch 17. The reason we do this is that way when something
does go wrong and something doesn't work it makes it much easier to find the
broken patch, you should never submit a patches that is broken even if it is
fixed directly after this breaks the assumption that every commit we make is
good and therefore when chasing a bug if a broken patch is found, is it likely
to be that bug, much easier to automate.

Your patch 28 that "Adds one more ssl test" should be squashed into the patch
adding the ssl tests, add them all once for this series. You can always more
work on more later but this series you have one patch adding ssl tests, have
all the ssl tests you're adding in that patch.

Hopefully that makes sense, please ask if I've just rambled!

Cyril

> https://github.com/openbmc/openbmc-test-automation/pull/1
> 
> Chris Austen (16):
>   Added DIMM specific inventory tests
>   base add for sensor tests
>   fixed review comments
>   created loops for tests
>   Add initial rest test cases     Support to test Null properties
>   Added full sensor suite
>   Update README.md
>   Handle new rest client from Brad
>   Added DIMM specific inventory tests
>   base add for sensor tests
>   fixed review comments
>   created loops for tests
>   Add initial rest test cases     Support to test Null properties
>   Added full sensor suite
>   Update README.md
>   Handle new rest client from Brad
> 
> Manjunath A Kumatagi (34):
>   Added git ignore file
>   Use argumnet file with pyrobot and added instructions to READEME file.
>   Update README for better html display
>   Added Inventory list testcase
>   fixed errors in tox and generate_argumentfile.sh script
>   Moved Read Attribute and Read Properties to rest_client
>   fix review comment
>   Added SSL tests
>   Ran the tests and fixed errors in SSL tests
>   Modified the doc
>   Added firmware version validation test
>   update readme file with tox version
>   Fix errors
>   Fix Errors in test_obmcrest.robot and removed the empty testcases.
>   Added one more testcase to test non-ssl connection to port 443
>   OpenBMC REST Testing
>   updated the REST API for firmware version
>   Added git ignore file
>   Use argumnet file with pyrobot and added instructions to READEME file.
>   Update README for better html display
>   Added Inventory list testcase
>   fixed errors in tox and generate_argumentfile.sh script
>   Moved Read Attribute and Read Properties to rest_client
>   fix review comment
>   Added SSL tests
>   Ran the tests and fixed errors in SSL tests
>   Modified the doc
>   Added firmware version validation test
>   update readme file with tox version
>   Fix errors
>   Fix Errors in test_obmcrest.robot and removed the empty testcases.
>   Added one more testcase to test non-ssl connection to port 443
>   OpenBMC REST Testing
>   updated the REST API for firmware version
> 
> Manjunath Kumatagi (2):
>   first commit
>   first commit
> 
> manjunath (6):
>   added files
>   added log statement
>   added testcase for etc passwd
>   added files
>   added log statement
>   added testcase for etc passwd
> 
> mkumatag (2):
>   Update README.md
>   Update README.md
> 
> 
> _______________________________________________
> openbmc mailing list
> openbmc at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc