(PDK-625) Formatting of modified status report and addition of full c… #365

HelenCampbell · 2017-11-30T10:46:59Z

…onvert report generation

coveralls · 2017-11-30T10:55:55Z

Coverage decreased (-0.5%) to 92.672% when pulling 99135c2 on HelenCampbell:report into 960c0ca on puppetlabs:master.

coveralls · 2017-11-30T11:35:01Z

Coverage decreased (-0.5%) to 92.672% when pulling e29ca85 on HelenCampbell:report into 960c0ca on puppetlabs:master.

coveralls · 2017-11-30T11:53:08Z

Coverage decreased (-0.5%) to 92.672% when pulling 44e0f35 on HelenCampbell:report into 960c0ca on puppetlabs:master.

coveralls · 2017-11-30T13:58:49Z

Coverage increased (+0.01%) to 93.148% when pulling 6458f0e on HelenCampbell:report into 960c0ca on puppetlabs:master.

…onvert report generation

coveralls · 2017-11-30T14:10:12Z

Coverage increased (+0.01%) to 93.148% when pulling bd95fa7 on HelenCampbell:report into 3fb9b91 on puppetlabs:master.

bmjen · 2017-12-01T00:13:31Z

Example of the summary:

[~/proj/sdk/test_modules/puppetlabs-motd]$ pdk convert --noop
-----------------------------------Files added----------------------------------
.gitlab-ci.yml
.pmtignore
.yardopts
spec/default_facts.yml
----------------------------------Files removed---------------------------------
---------------------------------Files modified---------------------------------
metadata.json
.gitignore
.rubocop.yml
.travis.yml
appveyor.yml
Gemfile
Rakefile
spec/spec_helper.rb
--------------------------------------------------------------------------------

coveralls · 2017-12-01T00:21:05Z

Coverage decreased (-0.7%) to 92.476% when pulling 326713e on HelenCampbell:report into 3fb9b91 on puppetlabs:master.

coveralls · 2017-12-01T00:26:33Z

Coverage increased (+0.06%) to 93.19% when pulling 3bafc0e on HelenCampbell:report into 3fb9b91 on puppetlabs:master.

coveralls · 2017-12-01T02:20:04Z

Coverage increased (+0.06%) to 93.19% when pulling 94d6877 on HelenCampbell:report into 3fb9b91 on puppetlabs:master.

DavidS · 2017-12-01T10:10:03Z

the empty "Files removed" section is confusing
looking at the middle part, it's not clear if the dashed line applies to the list below, or above. Adding an empty line before the heading could help:
A summary of the summary at the bottom, to reinforce what was shown.

[~/proj/sdk/test_modules/puppetlabs-motd]$ pdk convert --noop
-----------------------------------Files added----------------------------------
.gitlab-ci.yml
.pmtignore
.yardopts
spec/default_facts.yml

---------------------------------Files modified---------------------------------
metadata.json
.gitignore
.rubocop.yml
.travis.yml
appveyor.yml
Gemfile
Rakefile
spec/spec_helper.rb
--------------------------------------------------------------------------------

4 files added, 8 files modified.

bmjen · 2017-12-01T18:48:43Z

Updated output

[~/proj/sdk/test_modules/puppetlabs-motd]$ pdk convert --noop
-----------------------------------Files added----------------------------------
.gitlab-ci.yml
.pmtignore
.yardopts
spec/default_facts.yml
---------------------------------Files modified---------------------------------
metadata.json
.gitignore
.rubocop.yml
.travis.yml
appveyor.yml
Gemfile
Rakefile
spec/spec_helper.rb
--------------------------------------------------------------------------------
4 files added, 8 files modified.
You can find detailed differences in convert_report.txt.
[~/proj/sdk/test_modules/puppetlabs-motd]$ cd ../foo/
[~/proj/sdk/test_modules/foo]$ pdk convert --noop
No changes required.

coveralls · 2017-12-01T18:56:08Z

Coverage increased (+0.1%) to 93.232% when pulling 084c1ae on HelenCampbell:report into 3fb9b91 on puppetlabs:master.

bmjen · 2017-12-01T19:13:01Z

Latest update:

[~/proj/sdk/test_modules/puppetlabs-motd]$ pdk convert --noop

-----------------------------------Files added----------------------------------
.gitlab-ci.yml
.pmtignore
.yardopts
spec/default_facts.yml

---------------------------------Files modified---------------------------------
metadata.json
.gitignore
.rubocop.yml
.travis.yml
appveyor.yml
Gemfile
Rakefile
spec/spec_helper.rb

--------------------------------------------------------------------------------

4 files added, 8 files modified.
You can find detailed differences in convert_report.txt.

coveralls · 2017-12-01T19:21:14Z

Coverage increased (+0.1%) to 93.232% when pulling 7c41af9 on HelenCampbell:report into 3fb9b91 on puppetlabs:master.

scotje · 2017-12-01T19:29:08Z

lib/pdk/module/convert.rb

+      def self.print_summary(update_manager)
+        summary = {}
+        update_manager.changes.keys.each do |category|
+          update_category = update_manager.changes[category]


Can you simplify this to:

update_manager.changes.each do |category, update_category| update_category = update_category.keys if update_category.respond_to?(:keys) ... end

Then I think you could avoid the type checking conditional below.

Or if you want to avoid redefining updaate_category you could also do something like: update_files = update_category.respond_to?(:keys) ? update_category.keys : update_category

coveralls · 2017-12-01T20:05:11Z

Coverage increased (+0.08%) to 93.22% when pulling 3411ad4 on HelenCampbell:report into 3fb9b91 on puppetlabs:master.

scotje

🥂

coveralls · 2017-12-01T20:13:18Z

Coverage increased (+0.08%) to 93.216% when pulling 7a776df on HelenCampbell:report into 3fb9b91 on puppetlabs:master.

rickmonro · 2017-12-04T10:04:45Z

I'm late to the party here so won't request further changes at this point, and fwiw it looks great.

The only contribution I'll make when we get to a review of output consistency is to nudge the titles over to the left to allow for easier scanning.

[~/proj/sdk/test_modules/puppetlabs-motd]$ pdk convert --noop

--- Files added -----------------------------------------------------------------

.gitlab-ci.yml
.pmtignore
.yardopts
spec/default_facts.yml


--- Files modified -----------------------------------------------------------------

metadata.json
.gitignore
.rubocop.yml
.travis.yml
...

HelenCampbell force-pushed the report branch from 99135c2 to e29ca85 Compare November 30, 2017 11:27

HelenCampbell force-pushed the report branch from e29ca85 to 44e0f35 Compare November 30, 2017 11:44

HelenCampbell force-pushed the report branch 2 times, most recently from 30fa750 to 6458f0e Compare November 30, 2017 13:47

(PDK-625) Formatting of modified status report and addition of full c…

bd95fa7

…onvert report generation

HelenCampbell force-pushed the report branch from 6458f0e to bd95fa7 Compare November 30, 2017 14:02

bmjen requested a review from rodjek December 1, 2017 00:14

bmjen force-pushed the report branch from 326713e to 3bafc0e Compare December 1, 2017 00:18

bmjen force-pushed the report branch from 3bafc0e to 94d6877 Compare December 1, 2017 02:11

bmjen force-pushed the report branch from 94d6877 to 084c1ae Compare December 1, 2017 18:47

bmjen force-pushed the report branch from 084c1ae to 7c41af9 Compare December 1, 2017 19:12

scotje reviewed Dec 1, 2017

View reviewed changes

bmjen force-pushed the report branch from 7c41af9 to 3411ad4 Compare December 1, 2017 19:57

PDK-625) Added convert summary

7a776df

bmjen force-pushed the report branch from 3411ad4 to 7a776df Compare December 1, 2017 20:04

scotje approved these changes Dec 1, 2017

View reviewed changes

bmjen merged commit 9e1c643 into puppetlabs:master Dec 1, 2017

bmjen deleted the report branch December 1, 2017 21:27

bmjen restored the report branch December 1, 2017 21:27

bmjen added the feature label Dec 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(PDK-625) Formatting of modified status report and addition of full c… #365

(PDK-625) Formatting of modified status report and addition of full c… #365

HelenCampbell commented Nov 30, 2017

coveralls commented Nov 30, 2017

coveralls commented Nov 30, 2017

coveralls commented Nov 30, 2017

coveralls commented Nov 30, 2017

coveralls commented Nov 30, 2017

bmjen commented Dec 1, 2017 •

edited

Loading

coveralls commented Dec 1, 2017

coveralls commented Dec 1, 2017

coveralls commented Dec 1, 2017

DavidS commented Dec 1, 2017

bmjen commented Dec 1, 2017

coveralls commented Dec 1, 2017

bmjen commented Dec 1, 2017

coveralls commented Dec 1, 2017

scotje Dec 1, 2017

scotje Dec 1, 2017

coveralls commented Dec 1, 2017

scotje left a comment

coveralls commented Dec 1, 2017

rickmonro commented Dec 4, 2017

(PDK-625) Formatting of modified status report and addition of full c… #365

(PDK-625) Formatting of modified status report and addition of full c… #365

Conversation

HelenCampbell commented Nov 30, 2017

coveralls commented Nov 30, 2017

coveralls commented Nov 30, 2017

coveralls commented Nov 30, 2017

coveralls commented Nov 30, 2017

coveralls commented Nov 30, 2017

bmjen commented Dec 1, 2017 • edited Loading

coveralls commented Dec 1, 2017

coveralls commented Dec 1, 2017

coveralls commented Dec 1, 2017

DavidS commented Dec 1, 2017

bmjen commented Dec 1, 2017

coveralls commented Dec 1, 2017

bmjen commented Dec 1, 2017

coveralls commented Dec 1, 2017

scotje Dec 1, 2017

Choose a reason for hiding this comment

scotje Dec 1, 2017

Choose a reason for hiding this comment

coveralls commented Dec 1, 2017

scotje left a comment

Choose a reason for hiding this comment

coveralls commented Dec 1, 2017

rickmonro commented Dec 4, 2017

bmjen commented Dec 1, 2017 •

edited

Loading