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

backport, bugfix and new mate-menus api support #39

Merged
merged 1 commit into from May 31, 2018

Conversation

Projects
None yet
4 participants
@yetist
Copy link
Member

yetist commented Apr 26, 2018

Before that, I submitted a PR for mate-menus, if that PR merged, it will break mozo.

So now I've backport the alacarte code to mozo, and now it works good and some bug has been fixed, but more review and tests are needed, because this is a huge change.

@raveit65

This comment has been minimized.

Copy link
Member

raveit65 commented Apr 26, 2018

Without agreeing to this re-fork, translations are handle by transifex, so no need to change .po files
We submit changes to transifex server and the .po files will generate there after translations are done and we download changes before we release a new tarball.
Are you familiar 'with the 'git rebase -i last-good-commit-id' to change your commits?

@yetist

This comment has been minimized.

Copy link
Member Author

yetist commented Apr 26, 2018

I also updated the .po files, that include the new msgid from ui files, and some po has already translated, see here.

If you don't like the translation with PR, I can revert it, but the translator may have to work more.

@raveit65

This comment has been minimized.

Copy link
Member

raveit65 commented Apr 26, 2018

All, you changes will be overwritten if we generate a new resource mozo.pot for transifex.
So it is useless...

@yetist yetist force-pushed the yetist:backport branch from 0f311c8 to b3f7c99 Apr 26, 2018

@lukefromdc lukefromdc requested review from raveit65 , monsta and lukefromdc and removed request for raveit65 Apr 27, 2018

@lukefromdc
Copy link
Member

lukefromdc left a comment

I just found this version of Mozo works fine with the rest of these PRs in place, and displays the menu entires and can edit them same as before. This whole set of PRs looks good to go on this end, and solves our Python 2.7 problem

@raveit65
Copy link
Member

raveit65 left a comment

Works well with latest changes from mate-menus PR.

@lukefromdc

This comment has been minimized.

Copy link
Member

lukefromdc commented May 7, 2018

We do need to remember that this version of the Mozo PR requires the matching version of the mate-menus PR.

@@ -28,7 +28,7 @@ def main():
version = config.VERSION
except:
datadir = './data/'
version = '1.7.0'
version = '1.21.0'

This comment has been minimized.

@monsta

monsta May 10, 2018

Member

This is weird... no, not your changes. I mean, why do we have some fallback here? If we can't import config, the package is damaged, and we should just quit.

@monsta

This comment has been minimized.

Copy link
Member

monsta commented May 10, 2018

Hmm... what's the point of new LauncherEditor and DirectoryEditor?

@@ -18,9 +18,9 @@ AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE, "$GETTEXT_PACKAGE", [Gettext package])
AM_GLIB_GNU_GETTEXT
IT_PROG_INTLTOOL([0.40.0])

AM_PATH_PYTHON_VERSION(2.7, 2.7.0)
AM_PATH_PYTHON_VERSION(3.6, 3.6.0, 3.5, 3.5.0, 2.7, 2.7.0)

This comment has been minimized.

@monsta

monsta May 10, 2018

Member

Version 2.7 shouldn't be valid anymore after porting to Python 3.

I have a feeling we should just drop the whole acinclude.m4 which contains AM_PATH_PYTHON_VERSION, and just use AM_PATH_PYTHON.

@raveit65

This comment has been minimized.

Copy link
Member

raveit65 commented May 18, 2018

Looks like we have open questions here?

@monsta

This comment has been minimized.

Copy link
Member

monsta commented May 19, 2018

We also still have version bumps here and in mate-menus.

@monsta

This comment has been minimized.

Copy link
Member

monsta commented May 19, 2018

Wonder why we need a new mozo.svg image here...

@raveit65

This comment has been minimized.

Copy link
Member

raveit65 commented May 21, 2018

@yetist
Can you please revert version bump?
Last commit can be squashed?

@yetist

This comment has been minimized.

Copy link
Member Author

yetist commented May 21, 2018

@raveit65

Can you please revert version bump?
Last commit can be squashed?

This is done.

@yetist yetist force-pushed the yetist:backport branch from f647ca0 to b6f9cef May 21, 2018

@monsta

This comment has been minimized.

Copy link
Member

monsta commented May 21, 2018

I suggest keeping the old good mate-desktop-item-edit tool as menu item editor, like before.

@@ -22,8 +22,7 @@
import xml.dom.minidom
import xml.parsers.expat
import locale
import matemenu
from gi.repository import GLib
from gi.repository import MateMenu, GLib

This comment has been minimized.

@monsta

monsta May 25, 2018

Member

Maybe add gi.require_version here...

@monsta monsta requested review from vkareh and flexiondotorg May 25, 2018

@monsta

This comment has been minimized.

Copy link
Member

monsta commented May 25, 2018

Ok, asked for review from developers who probably know Python better than me 🙂

@raveit65

This comment has been minimized.

Copy link
Member

raveit65 commented May 27, 2018

Ok, asked for review from developers who probably know Python better than me 🙂

@vkareh @XRevan86
Can you please take a look at it ?

@monsta monsta referenced this pull request May 27, 2018

Closed

port to Python3 #41

@monsta

This comment has been minimized.

Copy link
Member

monsta commented May 27, 2018

Some parts of this can be squashed. For example, LauncherEditor and DirectoryEditor were first added then removed.

@lukefromdc

This comment has been minimized.

Copy link
Member

lukefromdc commented May 27, 2018

mate-menus is now merged, so this needs to be merged as well

@raveit65

This comment has been minimized.

Copy link
Member

raveit65 commented May 30, 2018

Some parts of this can be squashed. For example, LauncherEditor and DirectoryEditor were first added then removed.

Looks like this is gone, a 'grep' on the repo gives me no result.
Do we have gold status?

Support new mate-menus api
- Requires mate-menus 1.21.0
- Use Gobject-introspection
- Migrate to python3

@yetist yetist force-pushed the yetist:backport branch from 89d8ee5 to af19983 May 31, 2018

@monsta

This comment has been minimized.

Copy link
Member

monsta commented May 31, 2018

Ok, it's already squashed into one commit.

@monsta

monsta approved these changes May 31, 2018

Copy link
Member

monsta left a comment

With my limited Python knowledge, this looks ok.

@monsta monsta merged commit af19983 into mate-desktop:master May 31, 2018

@raveit65

This comment has been minimized.

Copy link
Member

raveit65 commented May 31, 2018

Hmm, why always 1 commit or too much commits?
Why not taken the middle? Lets say 5-6 for such a big change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment