fulcrologic / fulcro

A library for development of single-page full-stack web applications in clj/cljs
http://fulcro.fulcrologic.com
MIT License
1.55k stars 140 forks source link

Build the jar using Tools Build #504

Closed holyjak closed 2 years ago

holyjak commented 2 years ago

so that we can build the jar on the CI / GH Action.

holyjak commented 2 years ago

First step that will enable us to check Fulcro with cljdoc

awkay commented 2 years ago

This is going to need a lot more refinement/explination/documentation. I know you're trying for a GH action, but you've not stated a case around it, how it is supposed to work, how version numbers are supposed to work, etc. etc.

holyjak commented 2 years ago

I would be happy to revine, explain, and document this. What form do you prefer?

What I need from this is the ability to build the jar so that I can run cljdoc on it locally, to verify it works. I do not intend to publish or use further the file. (Though I could work on extending it so that it could replace your maven-based workflow, if you wanted.) Thus version does not really matter. Alternatively I could use mvn package but that requires that an additional dependency is installed in the GH Action image, which might be a problem. Using what we already have, ie Clojure CLI, seems simpler.

So the only intended usage is that a GH Action will eventually do something like

# inside fulcro/
clojure -Ttools install io.github.cljdoc/cljdoc-analyzer '{:git/tag "VERSION"}' :as cljdoc
clojure -T:build jar
clojure -Tcljdoc analyze-local # fails w/ non-0 exist status if a problem
awkay commented 2 years ago

So, on the one hand I do care the cljdoc publishes correctly, but on the other I don't want more to maintain. If the cljdocs are busted someone usually points it out and we fix it. The issues I have are:

So, TL;DR: I am probably not going to accept a PR that add cljdoc stuff to my repo.

holyjak commented 2 years ago

It seems to me we speak past each other. I am sorry for that, I will try to be clearer.

First of all, my intention was never to replace your current release process. You are obviously happy with it as it is, and that is perfectly fine. The only goal of the build.clj is to make it possible to build a throwaway fulcro jar for the purpose of checking that cljdoc can analyze it. I see how hardcoding the version in the build file could be confusing, so I now changed it to "snapshot" to make it clear that it does not matter. It is not ideal that the jar is built differently than fulcro actually is because there is a risk for a drift between the two but it is less of an disadvantage then having to install an extra tool to do the building for this purpose, IMO.

My experience with cljdoc is that it is very fragile, and I don't want to deal with it in my own project.

I understand that you had some bad experiences in CljDoc and I respect that. My experience is different, ever since I fixed it not to fail on unknown tagged literals. Since then, all the problems Fulcro had were missing or misplaced reader conditionals and the fix typically was wrapping something with #?(:cljs ...). So no big deal in my (limited) experience. However I understand that you have already more than enough on your plate and do not want to deal with anything more that is not essential.

For me cljdoc is rather important, I use it via Dash to get and browse offline docs. So I am willing to put in the work to make it functioning. We can well start but only checking cljdoc compatibility on pull requests, which you don't do on your changes and thus would not be impacted. Later we could enable the check to run also on releases or pushes to a branch with just a warning if it fails. Perhaps I could get it to send a notification to me if it fails.

awkay commented 2 years ago

I see. Yeah, the reason I spent so much time on cljdoc before is because I see it as valuable, but it was just too much of a pain back then. As long as what you're doing won't be something that is intended to change my current workflow, it's fine. Is this PR ready for that, and what else needs to be done?

holyjak commented 2 years ago

perhaps it makes sense to add the GH Action in the same PR

awkay commented 2 years ago

perhaps it makes sense to add the GH Action in the same PR

I would prefer to see the complete proposed soln, yes :)

awkay commented 2 years ago

Any news?

holyjak commented 2 years ago

Thank you for the reminder! I am working on it (https://github.com/cljdoc/cljdoc-check-action), waiting on a dependency, hopefully the action will be available soon and then I will finish this Pr at once.

holyjak commented 2 years ago

@awkay this Pr is now ready for your review. The GH Action for CljDoc has been added. Thank you for you patience!