Skip to content

Add section on tooling #18

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

Merged
merged 13 commits into from
Oct 22, 2020
Merged

Add section on tooling #18

merged 13 commits into from
Oct 22, 2020

Conversation

saulshanabrook
Copy link
Contributor

I wrote up some of the motivation and details behind the record-api tooling.

Happy to expand or reorganize from any feedback.

I didn't add many details about how the array-api-comparison works. @kgryte feel free to lemme know what I should add there, or add that in a follow up.

Copy link
Contributor

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

@saulshanabrook Nice draft! Left some comments (notably, need to turn on IDE spellcheck ;)).

I think a larger question concerns how much implementation detail we actually want to describe, given its likelihood to change and a desire to minimize spec "noise".

@saulshanabrook
Copy link
Contributor Author

Thanks @kgryte!

Nice draft! Left some comments (notably, need to turn on IDE spellcheck ;)).

Yes, need to do this! Was just writing in my browser...

I think a larger question concerns how much implementation detail we actually want to describe, given its likelihood to change and a desire to minimize spec "noise".

I took out some of the details, but I do think this will still need to evolve a bit as our data process continues to evolve.

@jorisvandenbossche
Copy link
Member

(sidenote: the aspects that are too detailed here, might be interesting to move to the python-record-api repo itself? Eg to its README or docs)

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @saulshanabrook, this looks like a good start to me.

Regarding structure, we have two tooling topics, so we need to decide whether to keep the structure of sections as it is now, or to split into the array-api-comparison and python-record-api parts right after the summary. I think I'd favour the latter.

There's probably a bit missing about how we use the data for decision making. That should come at the end I think, after it's clear to the reader what data is produced.

Also, can you wrap to 80 char lines? That will make it much easier to review.

@saulshanabrook
Copy link
Contributor Author

Regarding structure, we have two tooling topics, so we need to decide whether to keep the structure of sections as it is now, or to split into the array-api-comparison and python-record-api parts right after the summary. I think I'd favour the latter.

That makes some sense. I haven't started this yet.

There's probably a bit missing about how we use the data for decision making. That should come at the end I think, after it's clear to the reader what data is produced.

I added a few sentences on this, but wasn't very specific.

Also, can you wrap to 80 char lines? That will make it much easier to review.

Sure, wrapped.

@kgryte
Copy link
Contributor

kgryte commented Sep 17, 2020

@rgommers This should be ready for another round of reviews.

Copy link
Contributor Author

@saulshanabrook saulshanabrook left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@kgryte kgryte requested a review from rgommers October 8, 2020 08:00
@rgommers rgommers added the RFC Request for comments. Feature requests and proposed changes. label Oct 22, 2020
@rgommers rgommers force-pushed the saulshanabrook-patch-1 branch from 01ef7e2 to 69f0231 Compare October 22, 2020 11:48
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM too, just pushed a couple of tiny textual fixes. The story flows very well here, and the section ends up being nice and concise.

@rgommers rgommers merged commit 739e6a4 into master Oct 22, 2020
@rgommers rgommers deleted the saulshanabrook-patch-1 branch October 22, 2020 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for comments. Feature requests and proposed changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants