issue_comments

Table actions

1 row where author_association = "MEMBER", issue = 870619014 and user = 1217238 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

Suggested facets: updated_at (date)

These facets timed out: author_association, issue

user 1

  • shoyer · 1 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
834087321 https://github.com/pydata/xarray/pull/5234#issuecomment-834087321 https://api.github.com/repos/pydata/xarray/issues/5234 MDEyOklzc3VlQ29tbWVudDgzNDA4NzMyMQ== shoyer 1217238 2021-05-07T05:53:19Z 2021-05-07T05:53:19Z MEMBER

@andersy005 thanks for your hard work here. On the whole, I think this will be a net benefit to the codebase, but in my review I pointed out a number of locations where I did not find the style to be improved (in my opinion). There is also one place where I think your logic may have an error.

It would be nice to come up with a policy around "style clean-up" PRs for the future. In general, I think these are not a good idea, for several reasons:

  1. Even code that has been carefully checked can introduce bugs. Style clean-up PRs thus still need to be reviewed carefully.
  2. Fewer lines or tokens of code is not always better. Good examples from this PR include reduced nesting with early return statements, comprehensions and conditional expressions. All of these would be unobjectionable to use in new code (and are often a good idea), but code that doesn't use them even when it could is fine, too.
  3. Some ugly code works fine, and would not benefit from clean-up. For example, xarray.util.print_versions (which was forked from pandas) is quite ugly code. But it's peripheral to the project, and has barely been touched aside from automated reformatting.

Instead, I would suggest a policy of a selective cleanup only. Feel free to clean-up style when it arises in the process of normal coding, e.g., when you are modifying nearly or related code for another reason. This reduces the risk of introducing inadvertent errors, and the fact that you were working on nearby code means that this part of the codebase is worthy of improvement.

Code-base wide clean-up for particular issues may be warranted occasionally, but I would suggest always discussing it with other core developers first.

 
{
    "total_count": 2,
    "+1": 2,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Code cleanup 870619014

Advanced export

JSON shape: default, array, newline-delimited, object

CSV options: 

CREATE TABLE [issue_comments] (
   [html_url] TEXT,
   [issue_url] TEXT,
   [id] INTEGER PRIMARY KEY,
   [node_id] TEXT,
   [user] INTEGER REFERENCES [users]([id]),
   [created_at] TEXT,
   [updated_at] TEXT,
   [author_association] TEXT,
   [body] TEXT,
   [reactions] TEXT,
   [performed_via_github_app] TEXT,
   [issue] INTEGER REFERENCES [issues]([id])
);
CREATE INDEX [idx_issue_comments_issue]
    ON [issue_comments] ([issue]);
CREATE INDEX [idx_issue_comments_user]
    ON [issue_comments] ([user]);