Clojurians Log v2

@rahul080327 @borkdude let me know if my comments on the PR don't make sense. I've had to learn a lot about multi-platform docker builds lately and it is a lot 🙂

Its looking really nice @cap10morgan I'm learning about it too! knew a bit about manifest but mostly hid behind the buildx 😅 awesome stuff!

I'm just glad you guys are figuring this stuff out :-D

🙂 yeah I hadn't done much w/ manifest before either. I'm ripping it out of my PR now though haha

here goes nothin...

oh except it won't do anything interesting b/c of the branch filters. I'll let if run on this branch too for now.

there we go

Oh but it's still a snapshot so no joy

should I move the version back to 0.7.0 - does that help?

yeah that would let it trigger, but I think I need to do it on my branch. so probably easier if I just do it. BUT I'm nervous about letting it clobber those images on Docker Hub...

the docker token isn't exposed to pull requests anyway :)

ok good! 🙂

alright giving that a shot

hmm, got a test failure that seems unrelated / intermittent, but I don't have perms to re-run. I guess I can push another dummy commit though if you don't mind the clutter 🙂

pushed

oh yes, those hato tests use httpbin and that's a flaky service

hahaha, oh boy. the check in the .circleci/script/docker script is if [ -z "$CIRCLE_PULL_REQUEST" ] && [ "$CIRCLE_BRANCH" = "master" ] so it still didn't run

given that .circleci/config.yml already has branch filters and docker hub tokens aren't shared with pull requests, what do you think about just removing that?

I'm going to do it in this PR to test, but let me know if you'd like me to put it back before merging

why would we remove them? I don't want to push images in non-master branches?

the tokens aren't shared with PRs from outside the organization

Please make the PR as minimal as possible, not introducing any unnecessary changes

That's a good practice in general

yes, I agree of course. but I don't want to merge something that's untested!

these things are guarded against in other ways

but like I said, I'm happy to put it back before merging

yeah, I got it. for testing it's ok.

for example, the docker job won't even run in non-master branches due to the circleci config

hence why I had to whitelist my branch

maybe make a note in the PR as a TODO that we add it back before merging

sure thing

luckily the version change made it non-mergeable so you can't even do it accidentally now 😅

woof, more unrelated test failures

perhaps uncomment the tests for now - saves on circleci credits

I mean comment

also comment out the jvm stuff, etc, everything you don't need to for testing and to get faster feedback

I'll check back tomorrow. Thanks for looking into it!

sounds good I'll do all that if I get another failure

I was thinking we could have a skip-testsfilter to skip tests in branches if you're not interested in those

for intermediate commits

since they can take quite long

that's a good idea

haha and this time it got as far as not having docker hub creds which is pretty early in the process. I should probably go back to having it push to my docker hub account and add that to the pre-merge checklist

think I'm wrapping up for the day too, though