Closed tienne-B closed 2 years ago
I'll do it too
Hi, I can get this done.
Great! Don't hesitate to ask if you need anything!
Alright, the line is added and I manually checked it shows up where it should. I'm having a bit of a problem passing the django tests, as there are apparently other sessions using the test database. Should I keep trying to solve this or is the test suite broken?
That seems like an environment setup issue, if you can share the error message with us hopefully we can figure it out?
@rhe4n I think I remember hitting that error if I was running the Django server at the same time as running the tests
running the Django server at the same time
That was my first conclusion too, but after rebooting/restarting postgres service I'm still getting it.
if you can share the error message with us
Of course. Idk how you guys usually share logs this large, so in the meantime here you have the whole dj test
output:
As you can see, the tests first fail with no exception, but when the suite tries to delete the test database it crashes, as it is being used. No clue what else might be using this database (possibly a stray thread of this tests?).
Again, if this is not already a known problem I can probably figure it out on my own. The vm I'm using for this is a bit old, that might be the culprit.
Ah, yeah, that one's new to me, sorry.
For what it's worth, you'd be welcome to just submit a PR to see how the tests go; we have Django CI set up to run the test suite on all PRs. It'd be very surprising if something like this broke the regression tests, since I don't even think we have a test case for the tournament staff template.
Yeah, it makes no sense to me that such a small change might make the tests fail - I literally only changed one string. I will do the PR and we'll see what happens. On another note, how should I make the request? Should I finish my local feature and then PR develop -> develop? Or feature -> develop?
I just had a quick peek at your repo—if it's on your feature/equity-line
branch, then that seems like a great place to create the PR from.
It's your fork, so whether you merge it into your develop
branch is up to you—it won't affect this main repo. But I'd recommend against making the PR from your develop
branch, because whenever you push to the branch used to create the PR, the PR will automatically update (to track the branch). So it's better always to make PRs from feature branches.
If I can be a little pedantic about how the commits are phrased:
git rebase origin/develop
first to remove the merge commit 4c4f0b6
before creating the PR, so that the PR contains just one commit, that'd be even better.Thanks for your work on this!
Fixed in 7fa47eb7e2ff996b01171c05e1aad64d1719033d.
Knowing who is equity and how to contact them is important, but a line to list them is not in the default template, and so is less frequently done.
For this task, you'll need to add a line that asks to list Equity here:
https://github.com/TabbycatDebate/tabbycat/blob/3a9ca120dd30bf32a1e252a0b9f7add81694d970/tabbycat/tournaments/forms.py#L98-L100