Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Default verbosity to 1 for improved log data. Closes #441. #456

Merged
merged 3 commits into from
Nov 8, 2017
Merged

Default verbosity to 1 for improved log data. Closes #441. #456

merged 3 commits into from
Nov 8, 2017

Conversation

chambridge
Copy link

No description provided.

elyezer
elyezer approved these changes Nov 8, 2017
View reviewed changes
Copy link

@elyezer elyezer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elyezer
Copy link

elyezer commented Nov 8, 2017

Please make sure travis is happy again before merging

@coveralls
Copy link

coveralls commented Nov 8, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.09%) to 71.779% when pulling ad3faf4 on issues/441 into 4fada3d on master.

@elyezer
Copy link

elyezer commented Nov 8, 2017

I am a little bit concerned about the last changes to fix the failure on travis. So running with or without -v means the same. When -v is provided them if we want 1 by default it should be 2 then...

1 similar comment
@elyezer
Copy link

elyezer commented Nov 8, 2017

I am a little bit concerned about the last changes to fix the failure on travis. So running with or without -v means the same. When -v is provided them if we want 1 by default it should be 2 then...

@chambridge
Copy link
Author

@elyezer We are basically saying we want to always run ansible with -v whether provided or not. And if the user wants more they would need to go to -vv, but we stay in sync with the verbosity of ansible.

@elyezer
Copy link

elyezer commented Nov 8, 2017

So we should update the docs to state that, currently it states that if adding more -v then it will increase the verbosity and that is not happening for a single v.

@chambridge
Copy link
Author

@elyezer What would you suggest for the doc updates? Just the man page entry.

``-v``

  Enables the verbose mode. The ``-vvv`` option increases verbosity to show more information. The ``-vvvv`` option enables connection debugging.

@elyezer
Copy link

elyezer commented Nov 8, 2017 

``-v``

  Enables the verbose mode. The ``-vvv`` option increases verbosity to show more information. The ``-vvvv`` option enables connection debugging.

The option would be -vv (which is not a good practice IMO), and the description should say something like:

Increase the verbosity. The ``-vvv`` option increases verbosity to show more information. The ``-vvvv`` option enables connection debugging.

@coveralls
Copy link

coveralls commented Nov 8, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.05%) to 71.736% when pulling c4713b4 on issues/441 into 4fada3d on master.

@chambridge
Copy link
Author

@elyezer Made another update. Back to the first iteration, but updated the doc. Now our default is one verbosity ahead of ansible.

elyezer
elyezer approved these changes Nov 8, 2017
View reviewed changes
Copy link

@elyezer elyezer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elyezer
Copy link

elyezer commented Nov 8, 2017

I think in terms of usability this is better even though we are not "matching" ansible verbosity.

@chambridge chambridge merged commit 7ffe69b into master Nov 8, 2017
@chambridge chambridge deleted the issues/441 branch November 8, 2017 20:03
chambridge added a commit that referenced this pull request Nov 29, 2017
@chambridge
Merge branch 'master' into dev
6595d65 
* master:
  Add jboss.eap.init-files to JBOSS_FACTS. Closes #478. (#479)
  Add default dictionaries to handle lost host. Handle ansible rc if csv was successfully created. Closes #475. (#476)
  Fix addition of certs for wrong when condition. Closes #472. (#474)
  Present more status during discovery for larger environments. Closes #442. (#467)
  revert default logging change. Closes #460. (#461)
  Check for empty vault password. Closes #444. (#457)
  Highlight potential issues in the console output. Closes #443. (#458)
  Default verbosity to 1 for improved log data. Closes #441. (#456)
  Turn off colors in the ansible log. Closes #448. (#455)
  Fix task that can hang if systemctl paginates
  Update playbook to use free strategy. Closes #445. (#453)
  Merge 0.30 version into master branch (#451)

# Conflicts:
#	rho/ansible_utils.py
#	rho/facts.py
#	rho/inventory_scan.py
#	rho/postprocessing.py
#	rho/scancommand.py
#	roles/jboss_eap/tasks/main.yml
#	roles/write/tasks/main.yml
#	test/test_postprocessing.py
chambridge added a commit that referenced this pull request Nov 29, 2017
@chambridge
Merge branch 'master' into dev (#491)
91ae258 
* Merge branch 'master' into dev

* master:
  Add jboss.eap.init-files to JBOSS_FACTS. Closes #478. (#479)
  Add default dictionaries to handle lost host. Handle ansible rc if csv was successfully created. Closes #475. (#476)
  Fix addition of certs for wrong when condition. Closes #472. (#474)
  Present more status during discovery for larger environments. Closes #442. (#467)
  revert default logging change. Closes #460. (#461)
  Check for empty vault password. Closes #444. (#457)
  Highlight potential issues in the console output. Closes #443. (#458)
  Default verbosity to 1 for improved log data. Closes #441. (#456)
  Turn off colors in the ansible log. Closes #448. (#455)
  Fix task that can hang if systemctl paginates
  Update playbook to use free strategy. Closes #445. (#453)
  Merge 0.30 version into master branch (#451)

# Conflicts:
#	rho/ansible_utils.py
#	rho/facts.py
#	rho/inventory_scan.py
#	rho/postprocessing.py
#	rho/scancommand.py
#	roles/jboss_eap/tasks/main.yml
#	roles/write/tasks/main.yml
#	test/test_postprocessing.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@noahl noahl noahl approved these changes

@elyezer elyezer elyezer approved these changes

@kdelee kdelee Awaiting requested review from kdelee

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@chambridge @elyezer @coveralls @noahl