Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unecessary log messages #724

Merged
merged 1 commit into from Jun 3, 2019

Conversation

Projects
None yet
4 participants
@alexAubin
Copy link
Member

commented May 15, 2019

The problem

ynh_script_progression is awesome, but the info logging channel (what the regular user sees) is often kinda polluted with low-level technical messages, typically saying that service X is being reloaded...

E.g. :

$ yunohost app remove wordpress
Info: Removing application wordpress…
Info: [+...................] > Load settings
Info: [#+++++++............] > Remove dependencies
Info: [########+...........] > Remove the mysql database
Info: Removing database wordpress
Info: [#########+..........] > Remove app main directory
Info: [##########..........] > Remove nginx configuration
Info: Reload the service nginx
Info: [##########++........] > Remove php-fpm configuration
Info: /etc/php/7.0/fpm/conf.d/20-wordpress.ini wasn't deleted because it doesn't exist.
Info: Reload the service php7.0-fpm
Info: [############+++++...] > Remove fail2ban configuration
Info: Reload the service fail2ban
Info: [#################++.] > Remove the dedicated user
Info: Remove the user wordpress
Info: [####################] > Deletion completed

Imho those are duplicated / not necessary and the packager can see the detailed log if needed...

Solution

Remove those info messages

PR Status

Tested and working

How to test

Install / remove wordpress for example

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
@kay0u

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

I'm ok with it

@kay0u

kay0u approved these changes May 16, 2019

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

Some of them could be useful, maybe instead of just removing them, we could turn them to simple echo.
So it would be into the log if needed.

@alexAubin alexAubin added this to the 3.6.x milestone May 21, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented May 21, 2019

we could turn them to simple echo. So it would be into the log if needed.

Indeed, but my understanding is that all the info is already in the debug log anyway (since it logs all the call to the helpers etc)

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

That's right indeed.
Don't know. For me reading the trace of a helper isn't a problem. Don't know for others.

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

Sooo proposing to merge soon

@alexAubin alexAubin merged commit 61a9790 into stretch-unstable Jun 3, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alexAubin alexAubin deleted the unecessary-log-messages branch Jun 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.