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
IRCContact use the DATA API #1323
Conversation
As I said on IRC, in the future, please try to split the features (tags + IRC) in multiple PR, or at least in multiple commits. That's making it more difficult for us to review, and hence for you. For instance, it would allow you to get the tag feature in without having to wait for the IRC, or the contrary, and then just push a smaller PR behind that links both. Beside, the size of the commit does not make it easier to figure out which part of it broke that one feature once bisect told us it was this one ... That's another reason to prefer smaller 'focused' commits. There is still a long road ahead toward a working master, I'm well aware about that, but that's the reason we have to be specially careful about what's coming in. In order not to need to implement stuff twice. That being said, let's talk about the data model. I believe the question has to be asked about Tags table vs. Tags column (where the tags are stored as json string). Considering as column (and json) (as you did), do we want it to be nullable, or better not, and with a default of |
IRCContact use the DATA API See ticket:2714
(Oops) |
We probably will need to take it from here (new PRs), I somehow misread the comments. |
Well, I reverted the PR. @delanne You'd need to rebase the PR against latest master... Sorry about the mess. |
Hint: Take it as a chance to split your features ! ;) |
…act"" This reverts commit c203010.
Revert "Revert "Merge pull request #1323 from delanne/IRCContact""
IRCContact consume 2 events:
When we will be able to get the list of changes for a build, the IRCContact must be updated.