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] Simplify error management #574

Merged
merged 21 commits into from Dec 15, 2018

Conversation

Projects
None yet
5 participants
@irina11y
Copy link
Contributor

irina11y commented Nov 17, 2018

The problem

Utilisation trop complexe. Fait perdre du temps de dev.

Solution

Supression des code d'erreur "errno et de l'internationalisation "m18n.n"

@Josue-T

This comment has been minimized.

Copy link
Contributor

Josue-T commented Nov 17, 2018

As I'm working now on some new implementation, it could be good to know what will happen with this PR.

@zamentur

This comment has been minimized.

Copy link
Contributor

zamentur commented Nov 22, 2018

For information, this PR depends of a change on Moulinette.

I am ok on principle, but we need to test that correctly to avoid some fail due to typo issue...

Where are the Unit tests :p ?

@irina11y

This comment has been minimized.

Copy link
Contributor Author

irina11y commented Nov 25, 2018

@alexAubin alexAubin changed the title Enh Simplify moulinette error [enh] Simplify error management Nov 27, 2018

@zamentur
Copy link
Contributor

zamentur left a comment

I have reread this PR carefully, The are some change to do.

If youy prefer I can do it, but I believe you prefer to do it yourself

Show resolved Hide resolved src/yunohost/dyndns.py Outdated
Show resolved Hide resolved src/yunohost/user.py Outdated
Show resolved Hide resolved src/yunohost/user.py Outdated
Show resolved Hide resolved src/yunohost/user.py Outdated
Show resolved Hide resolved src/yunohost/user.py Outdated
Show resolved Hide resolved src/yunohost/utils/yunopaste.py Outdated
Show resolved Hide resolved src/yunohost/utils/yunopaste.py Outdated
Show resolved Hide resolved src/yunohost/utils/yunopaste.py Outdated

@irina11y irina11y force-pushed the irina11y:enh-moulinette-error branch from aa5baf3 to ee049ec Nov 30, 2018

Show resolved Hide resolved src/yunohost/settings.py Outdated
Show resolved Hide resolved src/yunohost/settings.py Outdated
Show resolved Hide resolved src/yunohost/settings.py Outdated
Show resolved Hide resolved src/yunohost/utils/yunopaste.py Outdated

@alexAubin alexAubin force-pushed the irina11y:enh-moulinette-error branch from 54f0103 to bd51a6f Dec 12, 2018

@alexAubin alexAubin force-pushed the irina11y:enh-moulinette-error branch from 17a682d to 18c33db Dec 12, 2018

alexAubin added some commits Dec 12, 2018

@alexAubin

This comment has been minimized.

Copy link
Member

alexAubin commented Dec 12, 2018

This should now be ready for final review (and careful final re-reading to spot any typo that would have been missed so far).

(The branch has been rebased to current stretch-unstable and misc fixes have been implemented, tested the whole thing with the unit/functional tests)

@alexAubin
Copy link
Member

alexAubin left a comment

Did a careful re-reading and found a few issue, that's good for me now 👍

@alexAubin alexAubin referenced this pull request Dec 13, 2018

Merged

Autopep8 #600

0 of 4 tasks complete
Show resolved Hide resolved src/yunohost/utils/error.py Outdated
@Psycojoker
Copy link
Member

Psycojoker left a comment

LGTM, thx <3

@Josue-T

This comment has been minimized.

Copy link
Contributor

Josue-T commented Dec 14, 2018

When this PR, is merged, it mean that all other PR will need to be fixed ?

@alexAubin

This comment has been minimized.

Copy link
Member

alexAubin commented Dec 14, 2018

When this PR, is merged, it mean that all other PR will need to be fixed ?

Yes, essentially you need to :

  • rebase (or merge stretch-unstable)
  • replace all MoulinetteError(m18n.n('some_key')) by YunohostError('some_key')
@Josue-T

This comment has been minimized.

Copy link
Contributor

Josue-T commented Dec 14, 2018

Yes, on my side it's ok but it just imply that some PR might be never updated and so never merged...

@Psycojoker

This comment has been minimized.

Copy link
Member

Psycojoker commented Dec 14, 2018

Yes, on my side it's ok but it just imply that some PR might be never updated and so never merged...

If a PR is never updated it will never been merged, this PR being done or not :/

And for this kind of small changes we can do the updates ourselves.

@alexAubin alexAubin merged commit 9106ee2 into YunoHost:stretch-unstable Dec 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment