crs-tools / tracker

CRS Ticket Tracker
Apache License 2.0
18 stars 11 forks source link

SQL changes workover #169

Closed a-tze closed 6 years ago

a-tze commented 6 years ago

See issue #166 .

Functionality testing still outstanding. Some minor, mostly cosmetic differences to current maser exist.

Feel free to add commits to this branch, I'll squash them in before removing WIP status. Accordingly, expect history rewrites in this branch as long as it is in WIP status.

The current last migration file will be replaced in a final commit when removing WIP state. If there are other migrations in the meantime, then a suitable change will be added.

pegro commented 6 years ago

First of all: thanks for (re-)working (on) this. Second: I would prefer keeping all these commits separate in the history and don't squash them. I think we aren't really bisectable anyway, so that shouldn't be a concern.

I'll try to comment on the other things inline.

a-tze commented 6 years ago

@pegro The "squashing in" did not mean I want to squash the commits/ the PR once it is approved. It's the solely purpose of this PR to introduce multiple commits with more explanation. What I would like to squash into other commits are the corrections that are done now, e.g. renaming the duplicate index or stuff like that, just to have a super clean history. Does that make sense?

pegro commented 6 years ago

Yes, makes sense. ;)

a-tze commented 6 years ago

Ok.. rebased to current master, added migration and did some testing. The branch now includes a somewhat consistent renaming to dependee and depender.

@pegro @jjeising please do a final review. Note: Github seems to get the order of commits wrong.

a-tze commented 6 years ago

@pegro please have a look at the last 3 commits from the latest batch. Another round of testing is still outstanding.

a-tze commented 6 years ago

@pegro good idea, I made this change for the function. Ommitting the change for the view right now.

Also @jjeising: If no objections exist, I will merge this in the next 3 days so that the other PRs can move forward.

pegro commented 6 years ago

Fine with me