I've worked on a bit of this.
Paul Wise <pabs@debian.org> writes:
> On Sun, Apr 26, 2015 at 1:40 AM, Simon Fondrie-Teitler wrote:
>
>> mediagoblin - web application for sharing photos, video and other media
>
> As I am potentially interested in mediagoblin as a possible front-end
> and or replacement for various debconf.org services, I've done a
> minimal first-pass review. I'm not involved in DebConf so that is of
> course dependent on the DebConf teams.
>
> The information in debian/copyright appears to be incorrect, most
> upstream files are AGPL not GPL.
Whoops, fixed.
> I would suggest packaging all the embedded code copies separately,
> including the one you added via a patch (which itself has an embedded
> code copy).
I looked into packaging them separately, but couldn't find good
documentation on how to package css or javascript files. A few Debian
developers I talked recommended not packaging them on their own.
> I would suggest using the new Files-Excluded option in
> debian/copyright and repacksuffix option in debian/watch, so that
> anyone can automatically convert the upstream tarball into the DFSG
> orig.tar.gz. You could also try to get those files removed from the
> upstream VCS.
>
> http://manpages.debian.org/man0/uscan
Cool, I didn't know about those options. I've added them.
I've talked to upstream about the files, and some will be removed in the
new release, like the binary translation files. Others need to be
there to make it easy for people installing via pip or a git checkout,
like the javascript.
> It would be great if the steps for setting up a mediagoblin instance
> documented in README.Debian could be converted into a script
> distributed upstream; the Debian package could then prompt the
> sysadmin via debconf for the relevant details (suggesting appropriate
> defaults in the text) and then run the script.
That's a good idea, although the version in README.Debian is closer to
an example of one possible setup, rather than a suggested way of running
it. I'm still thinking about the best way of doing this. I'm assuming
it's not a blocking issue for package upload?
> I'd suggest that debian/gmg.1 be generated at dh_auto_build time or
> from the upstream build system.
I tried doing this, but the script won't even succeed showing help
output when run without mediagoblin/tools/common.py somewhere on the
python path. I'm not sure if it's a good idea to modify the python path
during build.
> I'd use cgit for Vcs-Browser and https for both Vcs-* URLs.
Fixed.
> If it is possible to make MediaGoblin a Python 3 only package, I would
> suggest doing that.
Python3 support in mediagoblin is not great yet, though work is being
done to fix that. For now I think it makes most sense to leave it as python2.
> I note that upstream uses Transifex and Gitorious. The former is now
> proprietary and the latter is being shut down. Upstream may want to
> switch to something else for both.
They have already moved off Transifex and onto Pootle. Plans are being
made for a migration away from Gitorious.
> You may want to add an upstream metadata file:
>
> https://wiki.debian.org/UpstreamMetadata
I'll look into this.
> Automated checks:
>
> (I haven't built it or run lintian yet)
> https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git
>
> $ bashate --ignore E002
> ...
> 26 bashate error(s) found
>
> $ find -type f -iname '*.sh' -exec checkbashisms {} +
> could not find any possible bashisms in bash script
> ./devtools/update_translations.sh
> could not find any possible bashisms in bash script ./experimental-bootstrap.sh
>
> $ cme check dpkg
> ...
> Warning in 'control source Vcs-Browser' value
> 'http://anonscm.debian.org/gitweb/?p=collab-maint/mediagoblin.git':
> URL to debian system is not the recommended one (this can be fixed
> with 'cme fix' command)
> checking data
> Warning in 'patches:"0001-add-skeleton-manually-since-mediagoblin-depends-on-i.patch"
> Synopsis' value <undef>: Empty synopsis (this can be fixed with 'cme
> fix' command)
> ...
>
> $ codespell --quiet-level=3
> ./mediagoblin.ini:12: appropiate ==> appropriate
> ./paste.ini:75: accessable ==> accessible
> ./docs/source/index.rst:76: foreward ==> foreword, forward
> ./docs/source/api/media_interaction.rst:26: similiar ==> similar
> ./docs/source/api/oauth.rst:21: compatable ==> compatible
> ./docs/source/api/media.rst:44: similiar ==> similar
> ./docs/source/api/media.rst:142: similiar ==> similar
> ./docs/source/siteadmin/deploying.rst:159: prefered ==> preferred
> ./docs/source/siteadmin/deploying.rst:162: priviledges ==> privileges
> ./docs/source/siteadmin/relnotes.rst:141: existant ==> existent
> ./docs/source/siteadmin/relnotes.rst:548: Miscelaneous ==> Miscellaneous
> ./docs/source/pluginwriter/database.rst:27: appropiate ==> appropriate
> ./docs/source/pluginwriter/hooks.rst:31: commited ==> committed
> ./docs/source/pluginwriter/api.rst:233: tranform ==> transform
> ./debian/paste.ini:72: accessable ==> accessible
>
> $ find -empty
> ./extlib/pdf.js
> ./extlib/sandyseventiesspeedboat
> ./docs/source/_static/placeholder
> ./mediagoblin/tests/auth_configs/__init__.py
> ./mediagoblin/tests/fake_carrot_conf_empty.ini
> ./mediagoblin/tools/extlib/__init__.py
> ./mediagoblin/tools/__init__.py
> ./mediagoblin/init/celery/dummy_settings_module.py
> ./mediagoblin/oauth/tools/__init__.py
> ./debian/mediagoblin.lintian-overrides
>
> $ fdupes -q -r .
> ./mediagoblin/templates/mediagoblin/listings/collection.html
> ./mediagoblin/templates/mediagoblin/listings/tag.html
>
> ./mediagoblin/templates/mediagoblin/bits/above_content.html
> ./mediagoblin/templates/mediagoblin/bits/body_start.html
>
> ./mediagoblin/media_types/stl/migrations.py
> ./mediagoblin/media_types/pdf/migrations.py
> ./mediagoblin/media_types/image/migrations.py
> ./mediagoblin/media_types/audio/migrations.py
> ./mediagoblin/media_types/ascii/migrations.py
>
> ./mediagoblin/tests/test_submission/evil.png
> ./mediagoblin/tests/test_submission/evil
> ./mediagoblin/tests/test_submission/evil.jpg
>
> ./mediagoblin/user_pages/__init__.py
> ./mediagoblin/tests/testplugins/__init__.py
> ./mediagoblin/tests/fake_celery_module.py
> ./mediagoblin/edit/__init__.py
> ./mediagoblin/submit/__init__.py
> ./mediagoblin/federation/__init__.py
>
> ./mediagoblin/plugins/__init__.py
> ./mediagoblin/oauth/__init__.py
> ./mediagoblin/moderation/__init__.py
> ./mediagoblin/db/__init__.py
>
> ./licenses/AGPLv3.txt
> ./mediagoblin/plugins/archivalook/COPYING.txt
> ./mediagoblin/themes/airy/AGPLv3.txt
>
> ./licenses/CC0_1.0.txt
> ./mediagoblin/themes/airy/CC0_1.0.txt
>
> ./api-docs/source/themes/mg/static/mg.css
> ./docs/source/themes/mg/static/mg.css
>
> ./api-docs/source/themes/mg/static/logo_docs.png
> ./docs/source/themes/mg/static/logo_docs.png
>
> ./api-docs/source/themes/mg/theme.conf
> ./docs/source/themes/mg/theme.conf
>
> ./api-docs/source/themes/mg/layout.html
> ./docs/source/themes/mg/layout.html
>
> $ find -type f \( -iname '*.ttf' -o -iname '*.otf' -o -iname '*.sfd'
> -o -iname '*.pfa' -o -iname '*.pfb' -o -iname '*.bdf' -o -iname '*.pk'
> -o -iname '*.ttc' -o -iname '*.pcf' \) -exec fontlint {} \;
> <lots of output>
>
> $ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec POFileChecker {} +
> <lots of output>
>
> $ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec POFileSpell {} +
> <lots of output>
>
> $ find -type f \( -iname '*.po' -o -iname '*.pot' -o -iname '*.mo' -o
> -iname '*.gmo' \) -exec i18nspector {} +
> <lots of output>
>
> $ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec msgfmt
> --check --check-compatibility --check-accelerators
> --output-file=/dev/null {} \;
> <lots of output>
>
> $ pep8 --ignore W191 .
> <lots of output>
>
> $ pyflakes .
> <lots of output>
>
> $ find -type f -iname '*.sh' -exec shellcheck {} +
> <lots of output>
>
> $ grep -riE 'fixme|todo|hack|xxx' .
> <lots of output>
>
> --
> bye,
> pabs
>
> https://wiki.debian.org/PaulWise
Attachment:
signature.asc
Description: PGP signature