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

[enh] Reorganize helpers #717

Merged
merged 12 commits into from May 2, 2019

Conversation

Projects
None yet
3 participants
@alexAubin
Copy link
Member

commented Apr 25, 2019

The problem

Content of the various helper files is kind of a mess .. it's not clear where to find a given helper. The names are very ambiguous and kinda historical ... E.g. there is a dedicated file for mysql but backend contains stuff for systemd, php, fail2ban and logrotate all together :|

Solution

Reorganize the helpers and try to make everything a bit more semantic ... I've done various changes but basically we end up with the following (to be discussed) :

Individual files for interfaces with various technologies :

  • nginx
  • php
  • apt
  • mysql
  • postgresql
  • systemd
  • nodejs
  • fail2ban
  • logrotate

Yunohost-specific helpers so :

  • logging : print and exec helpers, related to debug/info/warn/critical levels
  • backup : all helpers related to backup (including the checksum things)
  • getopts : meta stuff for helper argument parsing
  • network : stuff related to network (ip, port)
  • setting : app settings, and also webpath_available/register which is kind of a setting ...
  • string : helpers to help with string (password generation, string normalization..)
  • user : manipulation of yunohost or system/unix users
  • utils : misc stuff that don't really fit in other files (setup_source, secure_remove, exit_properly ...) though to be discussed

PR Status

Not tested but uh, mainly to be discussed first

How to test

...

Validation

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

@alexAubin alexAubin changed the title Reorganize helpers [enh] Reorganize helpers Apr 25, 2019

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Apr 27, 2019

I know you really don't like that but we should really provide backward compatibility if we do that and it's not even that hard here: just replace function with echo "depreciated warning blablabla"; call_to_new_function $@ and in 1 or 2 releases we can just send massive automatically generated PR to applications, that would way less break user space :/

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2019

@alexAubin is that PR only about moving stuff from files to another ones ?
Hard to read the diff since there's a lot of modifications.

About moving helpers to other files, I agree.
And I'm not worried about backward compatibility, all apps are using /usr/share/yunohost/helpers so content of files doesn't matter.

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2019

Uh yea it's only about moving files (and I replaced some tabs with space because some stuff were a mess) so there shouldn't be any backward compatibility issue except for the initial sourcing of helpers ? (c.f. comment from Maniack)

Edit note : it's indeed quite difficult to read the diff, though reading commit by commit should be easier..

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2019

Not for apps at least. All apps are using /usr/share/yunohost/helpers, not a specific helper file.

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Apr 28, 2019

Oh ok, that's fine then :)

@alexAubin alexAubin added this to the 3.6.x milestone Apr 29, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Apr 29, 2019

Flagging as mergin soon then 😜

@alexAubin alexAubin merged commit 4411576 into stretch-unstable May 2, 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 reorganize-helpers branch May 2, 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.