royteeuwen commented on PR #40:
URL: 
https://github.com/apache/sling-org-apache-sling-committer-cli/pull/40#issuecomment-5290131647

   Thanks @rombert — all four points addressed in `819df9c`:
   
   | Comment | Change |
   |---|---|
   | reusable code out of `UpdateDistCommand` | new `DistRepository` in 
`org.apache.sling.cli.impl.dist`; commands call it instead of each other |
   | env var vs CLI argument | `--site-checkout` option, threaded as a 
parameter; env var removed entirely |
   | unrelated `DateProvider` change | it is a Sonar `java:S8688` fix (explicit 
time zone); explained inline, happy to split it out |
   | heavy mocking in the tests | both site test classes now run against a real 
repository via a `SiteRepository` rule |
   
   The mocking one was worth doing: the real-repository tests immediately 
caught that the commit was recording the wrong committer identity, which the 
mocked version could not see.
   
   Note the stack has grown a layer, since the review feedback sits below the 
shallow-clone work: #47 (merged) → #40 → #48.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to