Codecov Report

Merging #197 (4434ef6) into master (b72657c) will decrease coverage by 0.07%.
The diff coverage is 46.66%.

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
- Coverage   73.41%   73.34%   -0.08%     
==========================================
  Files          51       51              
  Lines        4300     4307       +7     
==========================================
+ Hits         3157     3159       +2     
- Misses        966      970       +4     
- Partials      177      178       +1     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b72657c...4434ef6. Read the comment docs.

Sorry for the delay in reply. I think this PR is on the right track but I would like to have some changes in the implementation.

• I don't like the fact that KubectlPristine, QbecPristine are now uppercase and acessible outside the package. This breaks encapsulation IMO.
• There is an existing mistake in the implementation of using the pristineReadWriter interface that includes both read and write. I think we just need 2 interfaces - one for read pristineReader and one for write pristineWriter. Apply and show only ever need to write and never have to read.
• The pristine package should have 2 functions NewReader and NewWriter. There is no read-writer that should be defined. The first is configured to read qbec/ kubectl/ fallback etc. and the second only ever writes so only the Qbec implementation is needed. That is, the pristine package determines how things are read and written and not the caller. This means that we can lowercase the K and Q in KubectlPristine and QbecPristine
• Now that you have moved all this functionality to a package called pristine - we should get rid of the stutter. So
  • pristine.Reader
  • pristine.Writer
  • qbec
  • kubectl
  • fallback
  • pristine.NewReader() and pristine.NewWriter

And please add a test to the show command to ensure that the annotation is set when the flag is passed in.

@kvaps do you plan to work on this? If not, I can implement it based on the feedback above. Let me know.

@gotwarlost, thanks for review!
Right now I'm little busy at work, so I have no much time for this, but I would like to return to this PR in a while.
In case if you want to implement this feature faster, you can do that by yourself, don't wait for me.

Any way I appreciate your notes, I'm glad to learning golang by participating this project 😉