home / github / issue_comments

Menu
  • Search all tables
  • GraphQL API

issue_comments: 834820456

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/pull/5234#issuecomment-834820456 https://api.github.com/repos/pydata/xarray/issues/5234 834820456 MDEyOklzc3VlQ29tbWVudDgzNDgyMDQ1Ng== 13301940 2021-05-07T22:20:00Z 2021-05-07T22:20:00Z MEMBER

@shoyer, Thank you for the thorough feedback.

Even code that has been carefully checked can introduce bugs. Style clean-up PRs thus still need to be reviewed carefully.

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.

👍🏽. I reverted back to the original style in most places. There're a few comments that I haven't addressed yet. I will look into those sometime this weekend.

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

I agree. I didn't expect the PR to get this large. My original intent when I created the PR was to clean the backends code, and I ended up invading other areas :(. In the future, I'll definitely make sure to start a discussion before working on changes that may end up affecting a large chunk of the code base.

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  870619014
Powered by Datasette · Queries took 0.642ms · About: xarray-datasette