Skip to content

Demo code review (shared)#1

Open
pkaminski wants to merge 9 commits intomasterfrom
work
Open

Demo code review (shared)#1
pkaminski wants to merge 9 commits intomasterfrom
work

Conversation

@pkaminski
Copy link
Member

@pkaminski pkaminski commented Sep 11, 2014

This is a simple pull request on a clone of a random repo, so you can play with Reviewable a bit. It's not complex enough to demo all the features but it'll get you started. Note that it's shared between all visitors to the site, so I reset the state the review's state every so often to keep it from getting out of hand.

You play the role of a reviewer—the butterflies will guide you through Reviewable's main features. In a real review, it would be up to the reviewee to fix the code and reply to the comments, iterating like this until all files are reviewed and all comments resolved. Unfortunately, this is as far as the demo goes... Try it for real on your own pull request!

This pull request is specially configured to not send comments to GitHub. Normally, published comments will appear in the GitHub pull request, but I've disabled the feature in this demo so that you don't end up being spammed by all future comments after you send one.


This change is Reviewable

@pkaminski
Copy link
Member Author

Comments from the review on Reviewable.io


astropy/coordinates/angles.py, line 146 [r1] (raw file):
Not sure if this is still needed?


astropy/coordinates/tests/test_sky_coord.py, line 153 [r5] (raw file):
Need to add more tests later.


@leereilly
Copy link

Comments from the review on Reviewable.io


astropy/coordinates/tests/test_sky_coord.py, line 153 [r5] (raw file):
reviewable.io is beautiful ! ❤️


@josephjang
Copy link

Comments from the review on Reviewable.io


astropy/coordinates/angle_utilities.py, line 162 [r5] (raw file):
There's a difference in the style. UINT vs. ufloat.


@bstrand
Copy link

bstrand commented Jan 26, 2015

Comments from the review on Reviewable.io


astropy/coordinates/angle_utilities.py, line 397 [r5] (raw file):
Would be helpful to describe the rounding strategy here.


@thockin
Copy link

thockin commented Feb 16, 2015

Comments from the review on Reviewable.io


astropy/coordinates/angle_utilities.py, line 152 [r5] (raw file):
I think you meant "cologne"


astropy/coordinates/angles.py, line 147 [r5] (raw file):
Clearly you meant to to fix this too.


@davidschreiber
Copy link

Review status: all files reviewed at latest revision, 33 unresolved discussions, all commit checks successful.


astropy/coordinates/angle_utilities.py, line 12 [r5] (raw file):
It would be way better to use bind this module to some custom name, e.g. using import os as customName


astropy/coordinates/angle_utilities.py, line 152 [r5] (raw file):
While I understand the need of a simple COLON here, I'd recommend we should refactor this to use a COLONY and some COLONISTS instead. This will prevent future COLONELS with cheap COLOGNEs to add wrong COLORS.


Comments from the review on Reviewable.io

@Reviewable Reviewable deleted a comment from jbuberel Dec 13, 2017
@Reviewable Reviewable deleted a comment from npatarino Dec 13, 2017
@Reviewable Reviewable deleted a comment from akrapfl Dec 13, 2017
@Reviewable Reviewable deleted a comment from akrapfl Dec 13, 2017
@Reviewable Reviewable deleted a comment from sankarsubramanian Dec 13, 2017
@Reviewable Reviewable deleted a comment from pnadar Dec 13, 2017
@Reviewable Reviewable deleted a comment from guruchahal Dec 13, 2017
@Reviewable Reviewable deleted a comment from tkaustin Dec 13, 2017
@Reviewable Reviewable deleted a comment from nikojohnarellano Dec 13, 2017
@Reviewable Reviewable deleted a comment from nicolas-brun Dec 13, 2017
@Reviewable Reviewable deleted a comment from nicolas-brun Dec 13, 2017
Copy link

@daiqiaobing daiqiaobing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

测试完成

@Reviewable Reviewable deleted a comment from rgolangh Nov 16, 2018
@Reviewable Reviewable deleted a comment from pacver Nov 16, 2018
@Reviewable Reviewable deleted a comment from pacver Nov 16, 2018
@Reviewable Reviewable deleted a comment from pacver Nov 16, 2018
@pkaminski pkaminski requested review from a team and removed request for a team January 10, 2019 06:36
Copy link

@muyehub muyehub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

123

Copy link

@staycolorful staycolorful left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11111

Copy link

@Claire-Ahn Claire-Ahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good

def p_colon(p):
'''
colon : sign UINT COLON UINT
colon : sign UINT COLON ufloat
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it consistent, UINT is all uppercase

@fahhem fahhem self-requested a review June 13, 2023 22:37
Copy link

@ajamzad ajamzad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just Checking the product.

Copy link

@hubblehox hubblehox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it consistent, UINT is all uppercase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.