Skip to content
This repository has been archived by the owner on Jan 25, 2019. It is now read-only.

Post a single comment for all browsers #4

Open
gsnedders opened this issue Feb 2, 2017 · 3 comments
Open

Post a single comment for all browsers #4

gsnedders opened this issue Feb 2, 2017 · 3 comments

Comments

@gsnedders
Copy link
Member

Similar to #3.

Ideally we should post a single comment with a single table per file which has columns for each different UA we're running the stability checker on. Once we add Edge and Safari having comments per UA is going to get even worse!

@bobholt
Copy link
Collaborator

bobholt commented Feb 2, 2017

I can get behind this. It would simplify the find/replace logic that makes #3 work (only allow the stability-bot user 1 comment, no matter the content), and remove the multiple POST/PATCH requests.

@jgraham
Copy link
Member

jgraham commented Feb 5, 2017

So there are a couple of reasons not to do this, as discussed on irc. The main one is that there is a huge complexity cost; we would either need to reparse the comments when updating results, or store all the data on the server ~indefinitely (at least until the PR is merged). Adding state to something that could be stateless is a big burden. The other problem is that github has a maximum comment size and some of our tables get pretty big; dumping the results of multiple browsers in a single comment could lead to unnecessary truncation.

I think the win here is disproportionately small compared to the benefit.

@bobholt
Copy link
Collaborator

bobholt commented Feb 6, 2017

After our IRC discussion, I agree.

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

No branches or pull requests

3 participants