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

Refactor gettext code #3737

Merged
merged 35 commits into from
Jul 11, 2021
Merged

Refactor gettext code #3737

merged 35 commits into from
Jul 11, 2021

Conversation

declension
Copy link
Member

  • Use pathlib paths everywhere. It's neater now and we get better checking
  • Add lots of typing, mypy does some work for us now
  • Use f-strings for great win
  • Fix some inappropriate test naming...
  • Improve some old / incorrect docstrings
  • Add some tests around setup.py PO commands (this broke them initially, but no tests broke)

Fixes #3736

 * Use `pathlib` paths everywhere. It's neater now and we get better checking
 * Add lots of typing, mypy does some work for us now
 * Use f-strings for great win
 * Fix some inappropriate test naming...
 * Improve some old / incorrect docstrings
 * Add some tests around `setup.py` PO commands (this broke them initially, but no tests broke)
For some reason other `.venv` is included (in Fedora...)
 * Enforce user at Docker level not script
 * Exit on failure
 * Use same pytest options as other jobs
 * Add steps in Github like other jobs
* Fedora - don't install ql into venv
* Nicer debug on failure though
@declension declension merged commit 3ea15c0 into master Jul 11, 2021
@declension declension deleted the update-gettext-code branch July 11, 2021 16:12
@X-dark
Copy link

X-dark commented Jul 12, 2021

Seems I cannot build with that change. Is this maybe because of a missing dep?

Traceback (most recent call last):
  File "/build/quodlibet-git/src/quodlibet-git/setup.py", line 131, in <module>
    main()
  File "/build/quodlibet-git/src/quodlibet-git/setup.py", line 127, in main
    setup(**setup_kwargs)
  File "/usr/lib/python3.9/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/usr/lib/python3.9/distutils/dist.py", line 966, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python3.9/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "/usr/lib/python3.9/distutils/command/build.py", line 135, in run
    self.run_command(cmd_name)
  File "/usr/lib/python3.9/distutils/cmd.py", line 313, in run_command
    self.distribution.run_command(command)
  File "/usr/lib/python3.9/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "/build/quodlibet-git/src/quodlibet-git/gdist/po.py", line 260, in run
    basepath = self.build_base / "share" / "locale"
TypeError: unsupported operand type(s) for /: 'str' and 'str'

@declension
Copy link
Member Author

Thanks @X-dark, my bad; guess I missed those. Moved to #3743

declension added a commit that referenced this pull request Jul 12, 2021
* Broken in refactoring in #3737
* Add / correct type hints
* Add integration tests for these commands too

Fixes #3743
declension added a commit that referenced this pull request Jul 12, 2021
* Broken in refactoring in #3737
* Add / correct type hints
* Add integration tests for these commands too

Fixes #3743
@lazka
Copy link
Member

lazka commented Sep 9, 2021

this made paths in quodlibet.pot absolute somehow

@lazka
Copy link
Member

lazka commented Sep 9, 2021

fixed in #3798

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Fedora job now breaking on xgettext issues
3 participants