Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL comments? #378

Open
AlexBaranosky opened this issue May 12, 2017 · 16 comments
Open

SQL comments? #378

AlexBaranosky opened this issue May 12, 2017 · 16 comments

Comments

@AlexBaranosky
Copy link
Member

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:

threatgrid@020ccb8

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

@AlexBaranosky
Copy link
Member Author

@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
Copy link
Member

immoh commented May 18, 2017

@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
Copy link
Member Author

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

@immoh
Copy link
Member

immoh commented May 26, 2017

@AlexBaranosky Implementation looks good, name is fine too.

@AlexBaranosky
Copy link
Member Author

@immoh PR created: #380

@AlexBaranosky
Copy link
Member Author

AlexBaranosky commented May 26, 2017

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
Copy link
Member Author

fixed, and merged #380

@AlexBaranosky
Copy link
Member Author

AlexBaranosky commented Jun 1, 2017

@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
Copy link
Contributor

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
Copy link
Member

immoh commented Feb 28, 2018

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:

  • Create a commit that contains:
    • Version update in project.clj from (0.4.4-SNAPSHOT to 0.4.4)
    • Version update in README.md
    • Generated Codox docs
    • Release notes in HISTORY.md
  • Deploy new jar to Clojars (lein deploy clojars?)
  • Tag commit with v0.4.4
  • Create a new commit in gh-pages branch that updates the docs (visible in http://korma.github.io/Korma/)

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
Copy link
Contributor

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
Copy link
Member

immoh commented Feb 28, 2018

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
Copy link
Contributor

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
Copy link
Member

immoh commented Feb 28, 2018

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
Copy link
Member

immoh commented Feb 28, 2018

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants