Closed surprisetalk closed 9 months ago
cc @gregorybchris for thoughts
Overall it's a cool validation of the pygit2 work and I'm excited to see that develop
This is kind of a major syntax change. I think @ is better suited for indexing (even is "at" in English) than :: is. Is there another char we can use for pinning? Perhaps ~? I think this language change should also be pulled out of this PR and discussed separately
Yeah, I'd be fine pulling that out into a separate PR 👍🏼 I think you'll appreciate the symmetry between #
and ::
, plus how it relates alts and records together nicely. I'll explain in the new pr
For now, pinning can be ^^
@gregorybchris I think I'm a bit out of my depth here trying to get this cryptography package to build. Is this something you have experience with?
@gregorybchris I think I'm a bit out of my depth here trying to get this cryptography package to build. Is this something you have experience with?
Is the issue that you want to install it in Docker and have it available to scrapscript.py in the python environment? And right now we're not doing an install of dependencies from pyproject.toml?
There are two separate issues:
We can't run the unit tests because cffi
is not working as expected with GitHub actions, which is a dependency of cryptography
. The tests run fine on my machine, but they're failing on actions. Other people report needing to uninstall reinstall when using Linux, but I haven't found the magic incantation yet.
We can't build the Docker image because of the cryptography
dependency, because it's not being bundled into the binary. I couldn't figure out how to get it working because of some weird SSL stuff.
Hmm. Actually, I'm going to sidestep these issues and make cryptography
an optional dependency, like we're doing with git. We can figure out the CI stuff in a follow-up PR
Closing until we have a real design discussion about implementation :)
I'll comment a little more later (probably in depth after next Monday; away this weekend) but my surface-level comment is:
This is kind of a major syntax change. I think
@
is better suited for indexing (even is "at" in English) than::
is. Is there another char we can use for pinning? Perhaps~
? I think this language change should also be pulled out of this PR and discussed separately