search

korma / Korma

Tasty SQL for Clojure.
http://sqlkorma.com
1.48k stars 222 forks source link

SQL comments? #378

Open AlexBaranosky opened 7 years ago

AlexBaranosky commented 7 years ago

For work I added a feature to add comments, so my ops guys can get better debugging info for the sql queries. Let me know if this is interesting to consider for the main fork:

https://github.com/threatgrid/Korma/commit/020ccb8b1160f38b8645be2bb173b5fe9aa8c617

Then I pull process/thread/ring-request info to populate for each query.

AlexBaranosky commented 7 years ago

@immoh are you in charge? I probably still have commit rights, but would love to get feedback on this quick branch I hacked up pretty fast.

immoh commented 7 years ago

@AlexBaranosky I am still in charge but there hasn't been much activity on Korma lately (latest release is almost 1 year ago).

This seems like something that could be useful to other people so I am ok with taking it in.

AlexBaranosky commented 7 years ago

@immoh any considerations about the fn name add-comment or anything in the implementation? Then I can PR it.

immoh commented 7 years ago

@AlexBaranosky Implementation looks good, name is fine too.

AlexBaranosky commented 7 years ago

@immoh PR created: https://github.com/korma/Korma/pull/380

AlexBaranosky commented 7 years ago

hmmm let me fix that... I guess there was a big change between my branch and master where I rebased it... or maybe I just screwed up the rebase :)

AlexBaranosky commented 7 years ago

fixed, and merged #380

AlexBaranosky commented 7 years ago

@immoh what's the process for doing a Korma release... I forget, it's been a while. Is it documented at all?

git diff master v0.4.3 is not a small amount of changes. Maybe it's time to release 0.4.4?

venantius commented 6 years ago

I gave Chris a ping on Twitter to ask about release processes. I agree that we should have an 0.4.4 release soon; it's rarely a good idea to accumulate too many commits between releases.

immoh commented 6 years ago

I was supposed to reply to this long time ago but somehow it slipped my mind. I have been doing releases in the following way:

However, there’s a problem with current master that it may not be in a releasable state even though tests are passing. There were some pretty big changes in master there were not 100% completed and I made release v0.4.3 from separate branch where I cherry-picked some changes from master. Remaining work for 0.5.0 was tracked here: https://github.com/korma/Korma/milestone/2. This work needs to be completed.

Another option would be to revert all changes from master that have been done before v0.4.3 release but are not included in the release.

venantius commented 6 years ago

Ah. Hm.

I don't love the idea of having master being unclean or deviant from the actual releases. I'm not sure what the right approach should be here. My naive suggestion would be that we just mark master head as a release candidate for 0.5.0 and release it as 0.5.0-rc1 and try to get feedback from the community about whether anything breaks for them. That gives us a bit of room to address issues and will increase our confidence about being able to release 0.5.0 safely.

313 is the only issue currently blocking on the 0.5.0 milestone - is that an absolutely necessary issue for the release?

immoh commented 6 years ago

Yes, definitely a bad decision on my side to have master unclean.

I cannot remember all the details as it has been a while since I worked on Korma. I looked at the closed issue in the milestone and the big change in 0.5.0 is removal of global options (breaking change). This can cause wrong delimiters being used in certain situations and #313 is about this. It is hard to say if it's absolutely necessary. I think cutting a release candidate and getting feedback from the community sounds like a reasonable approach.

venantius commented 6 years ago

I don't think I have release rights in Clojars - do you might actually cutting the release? I'm happy to handle messaging to Clojurians, the Clojure/Korma Google groups, etc.

immoh commented 6 years ago

Sure, I will try to find time to do it later today.

@AlexBaranosky you are also admin of Clojars korma group - could you add @venantius to the group?

immoh commented 6 years ago

0.5.0-RC1 is now published to Clojars. I didn't change version in README, probably it makes sense to mention both last stable version and release candidate versions.

venantius commented 6 years ago

I've now sent the email to the Clojure google group, the Korma google group, and Clojurians.