metabase / toucan

A classy high-level Clojure library for defining application models and retrieving them from a DB
Eclipse Public License 1.0
570 stars 49 forks source link

Added models/primary-key #53

Closed axrs closed 5 years ago

axrs commented 5 years ago

Added in support for changing the primary key, https://github.com/metabase/toucan/issues/3

Thanks for contributing to Toucan. Before open a pull request, please take a moment to:

Once you've done all that, open a PR! Make sure to at-mention @camsaul in the PR description. Otherwise I won't get an email about it and might not get review it right away. :)

Thanks for your contribution!

AndreTheHunter commented 5 years ago

Can you please carefully go thru your PR and remove all the changes to the CI config and other unrelated changes. It looks to me like you might just need to rebase your commit off latest master.

The changes in the CircleCI yaml config allows allows us to deploy our own fork. See https://circleci.com/gh/jesims/workflows/toucan deploying to https://clojars.org/io.jesi/toucan. I've made that configuration environment variable driven as not break anything in metabase/toucan. Since we need our changes in production, we can't always wait for it to be merged into the upstream.

I've checked, and this branch is up to date with master. I'd recommend configuring this repository and setting master as a protected branch. Go to https://github.com/metabase/toucan/settings/branches. See https://help.github.com/articles/enabling-required-status-checks/ for more info.

Happy to discuss any changes/alternatives 😃

camsaul commented 5 years ago

I don't want to merge those changes you've made to the CI config or project.clj into the core Toucan repo

AndreTheHunter commented 5 years ago

@camsaul Those changes wouldn't break anything but OK. I've added some comments outlining the changes to project.clj

camsaul commented 5 years ago

Thanks @AndreTheHunter. Looks good.

camsaul commented 5 years ago

Merged. I'll update documentation and other things in a few days and push out a new release if all looks good

camsaul commented 5 years ago

Pushed out release 1.11.0 which includes this PR. Thanks again