Hey there,
short update (since I paused due to birth of my child).
I rebased on 522a61b24a1b7767682eaf7b29c59e40a0a9b73f /
https://git.savannah.gnu.org/cgit/mediagoblin.git/commit/?id=522a61b24a1b7767682eaf7b29c59e40a0a9b73f
You can see a comparison at
https://jaenis.ch/hobbies/coding/repos/ryuno-ki/mediagoblin/compare/main...d2254dcdb0
(there's a bug in Gitea I guess, which fails over the slash in
my
branch name - I'm replying here once I made progress and will
update
the link).
Once I feel confident to wrap it up, I plan to squash the
commits
(unless there is another process defined) to make it easier to
integrate the changes (as a single commit).
Can you send me details of the failing tests please and the
commands
you used to run them?
I used ./runtests.sh but that failed with a cryptic message
($VENVS is
the place where I put all my Python projects. The git repo is
cloned
into "src"):
Using ./bin/py.test
+ exec ./bin/py.test -x ./mediagoblin/tests --boxed
Traceback (most recent call last):
File "$VENVS/mediagoblin/src/bin/py.test", line 33, in
<module>
sys.exit(load_entry_point('pytest', 'console_scripts',
'py.test')())
File "$VENVS/mediagoblin/src/bin/py.test",
line 22, in importlib_load_entry_point
for entry_point in distribution(dist_name).entry_points
File
"/usr/lib/python3.7/site-packages/importlib_metadata/__init__.py",
line
558, in distribution return
Distribution.from_name(distribution_name)
File
"/usr/lib/python3.7/site-packages/importlib_metadata/__init__.py",
line
215, in from_name
raise PackageNotFoundError(name)
importlib_metadata.PackageNotFoundError: No package metadata
was
found for pytest
Funnily enough, I can execute ./bin/py.test ./mediagoblin/tests/
-x
once I switched into the repo. Without a bail (-x) I run into
resource
exhaustion at about 42% of execution.
======================== test session starts
=========================
platform linux -- Python 3.7.10, pytest-6.2.5, py-1.11.0,
pluggy-1.0.0
rootdir: $VENVS/mediagoblin/src/mediagoblin/tests, configfile:
pytest.ini plugins: celery-4.2.2 collected 182 items / 3 skipped
/ 179
selected
mediagoblin/tests/test_api.py ......................
mediagoblin/tests/test_audio.py F
============================== FAILURES
===============================
___________________________ test_transcoder
___________________________
Traceback (most recent call last):
File "$VENVS/mediagoblin/src/mediagoblin/tests/test_audio.py",
line
75, in test_transcoder with create_data_for_test() as
(audio_name,
result_name): File "/usr/lib/python3.7/contextlib.py", line 112,
in
__enter__ return next(self.gen)
File "$VENVS/mediagoblin/src/mediagoblin/tests/test_audio.py",
line
65, in create_data_for_test with create_audio() as audio_name:
File "/usr/lib/python3.7/contextlib.py", line 112, in
__enter__
return next(self.gen)
File "$VENVS/mediagoblin/src/mediagoblin/tests/test_audio.py",
line
48, in create_audio
pipeline.add(enc)
TypeError: Argument 1 does not allow None as a value
======= 1 failed, 22 passed, 3 skipped, 6 warnings in 53.78s
========
I took the liberty to add a new directory for testing JavaScript
in the
browser using the Jasmine testing framework (MIT-licensed):
https://jasmine.github.io/api/3.10/global
This way, I can write a spec for the existing implementation
before
rewriting it in more modern syntax. In order to run, you start a
web
server (e.g. python3 -m http.server) and navigate to the
SpecRunner.html. I recommend to do so higher up in the directory
tree
since I load the audio.js via directory traversal and the file
has to
be found. The tests are then executed in the web browser.
Let me know if this is something you would be interested in
adopting.
According to
https://www.gnu.org/licenses/license-list.html#X11License
it is compatible to GPL.
We may want to move the <script src="../audio.js"> to the end
of the
page body and drop all the ready() code.
I introduced some new blocks to collect them at the end (see the
base.html) called mediagoblin_scripts. It would be great if you
could
already provide that upstream (even if empty).
For the sake of readability (and as experienced templater) I can
recommend to name the end-block tags:
https://jinja.palletsprojects.com/en/3.0.x/templates/#named-block-end-tags
I haven't grokked the plugin logic yet so added a `defer`
attribute
there for now:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#attr-defer
This requires testing.
Feel free to consider the spirit of the code, rather than doing
a
literal translation.
[…]
Feel free to translate any code to use ES6 features where they
make
things easier to read.
I wasn't exactly sure how far I can push this. `const` and `let`
should
be safe enough by now: https://caniuse.com/const
ES6 class syntax is not supported in Internet Explorer and Opera
Mini:
https://caniuse.com/es6-class
Would that pose a problem?
Some Node.js / Frontend-Tooling can understand
https://browserslist.dev/ if you are looking for a way to
document the
supported browsers.
I'd suggest adding a "use strict"; at the top.
Included (but not pushed yet). Thanks!
Fade and other animations are absolutely not critical and
you're
welcome to just drop them if they're not trivial to implement.
I'm looking into it once I progressed on the tests. Ideally, I
would
use CSS Animations similar to Svelte:
On Mon, 30 Aug 2021 12:00:16 +1000
Ben Sturmfels <ben@sturm.com.au> wrote:
Hi Andre,
Woo! Great work on porting audio.js! Comments inline below.
On Sat, 28 Aug 2021, Andre Jaenisch wrote:
> Hey Ben,
>
> I've got started by replicating the repository on my private
> Gitea
> instance. (I don't accept other accounts there, so don't look
> for a
> sign-up page - in that sense, it is private).
>
> Next, I ran the tests (still five failing) and branched off
> to
> tackle the first porting in
> https://jaenis.ch/hobbies/coding/repos/ryuno-ki/mediagoblin/src/branch/refactor/port-audio-js
>
Can you send me details of the failing tests please and the
commands
you used to run them?
> The intermediate changes can be seen in:
>
https://jaenis.ch/hobbies/coding/repos/ryuno-ki/mediagoblin/compare/712728c331ea3518b1964bbc7fcf06ff2c567064...c00d3ad053d5d0862f1b71ec126e3c9e1493967d
>
Great stuff. A couple of comments:
We may want to move the <script src="../audio.js"> to the end
of the
page body and drop all the ready() code.
Feel free to consider the spirit of the code, rather than doing
a
literal translation. For example, the
"spectrogram.addEventListener"
code is doing some pretty complicated DOM traversal. I suspect
this
could be simplified with:
var seekbar = document.querySelector('.audio-spectogram
.seekbar');
seekbar.addEventListener(...
Fade and other animations are absolutely not critical and
you're
welcome to just drop them if they're not trivial to implement.
Feel free to translate any code to use ES6 features where they
make
things easier to read.
I'd suggest adding a "use strict"; at the top.
>
> In order to continue on audio.js I need some assistance. The
> file
> gets referenced in these places:
>
> -
>
extlib/sandyseventiesspeedboat/templates/mediagoblin/media_displays/audio.html
> -
>
mediagoblin/plugins/archivalook/templates/archivalook/feature_displays/audio_head.html
> - mediagoblin/templates/mediagoblin/media_displays/audio.html
>
> Which of them are relevant?
> How can I navigate to them?
> I want to make sure to not break something, but have to have
> a look
> at the HTML to properly port the code (there are difference
> between
> a single element being selected vs. multiple).
>
> My gut feeling would be to look at the templates one and
> ignore the
> other.
Yes, as you suggest, ignore the "sandyseventiesspeedboat"
template,
since this is external to the project (using git submodule) and
no
longer maintained. But change the other two. Archivalook is a
plugin
that you'll need to enable to test.
Regards,
Ben