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

Adding missing docs and keywords for TableRow. #4333

Merged
merged 2 commits into from Nov 10, 2016

Conversation

Projects
None yet
3 participants
@suheb
Contributor

suheb commented Feb 27, 2016

Fixes #4332
Signed-off-by: Suhaib Khan suheb.work@gmail.com

Adding missing docs and keywords for TableRow.
Signed-off-by: Suhaib Khan <suheb.work@gmail.com>
@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry May 8, 2016

Member

@REAS This is one to be added to your keywords generator (rather than merged into keywords.txt).

Member

benfry commented May 8, 2016

@REAS This is one to be added to your keywords generator (rather than merged into keywords.txt).

@REAS

This comment has been minimized.

Show comment
Hide comment
@REAS

REAS May 9, 2016

Member

This is a quick, easy fix, but it's a slippery slope that I worry about. I don't want to make it difficult, but we have many "advanced" methods across many classes that aren't in the simplified reference. Right now, all of the color coding in the PDE comes from this simplified reference list or the hardwired items. If we add these to the hardwired items, there are additional dozens that should be added too. Is that what we want to do? It's making more sense, the more I think about it.

Member

REAS commented May 9, 2016

This is a quick, easy fix, but it's a slippery slope that I worry about. I don't want to make it difficult, but we have many "advanced" methods across many classes that aren't in the simplified reference. Right now, all of the color coding in the PDE comes from this simplified reference list or the hardwired items. If we add these to the hardwired items, there are additional dozens that should be added too. Is that what we want to do? It's making more sense, the more I think about it.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry May 9, 2016

Member

Yep, agreed. getColumnCount() and getColumnTitle() are the ones to include in the reference, the others all belong in the javadoc.

But a PR that improves the javadoc for the others would be welcome.

Member

benfry commented May 9, 2016

Yep, agreed. getColumnCount() and getColumnTitle() are the ones to include in the reference, the others all belong in the javadoc.

But a PR that improves the javadoc for the others would be welcome.

@suheb

This comment has been minimized.

Show comment
Hide comment
@suheb

suheb May 12, 2016

Contributor

@benfry Should I remove these methods from keywords.txt? Also, what the correct place to add keywords then?

Contributor

suheb commented May 12, 2016

@benfry Should I remove these methods from keywords.txt? Also, what the correct place to add keywords then?

@REAS

This comment has been minimized.

Show comment
Hide comment
@REAS

REAS May 23, 2016

Member

This is the correct spot: https://github.com/processing/processing-docs/blob/master/generate/keywords_base.txt

The keywords.txt file is generated, it shouldn't be edited directly.

Member

REAS commented May 23, 2016

This is the correct spot: https://github.com/processing/processing-docs/blob/master/generate/keywords_base.txt

The keywords.txt file is generated, it shouldn't be edited directly.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Nov 10, 2016

Member

@suheb Would you like to update this to add those two keywords to keywords_base.txt, remove them from keywords.txt and re-submit this patch, or should I close it?

Member

benfry commented Nov 10, 2016

@suheb Would you like to update this to add those two keywords to keywords_base.txt, remove them from keywords.txt and re-submit this patch, or should I close it?

@suheb

This comment has been minimized.

Show comment
Hide comment
@suheb

suheb Nov 10, 2016

Contributor

@benfry Ohh, sorry about this. I thought I'd fixed this. Will update this PR and open a new one in processing-docs.

Contributor

suheb commented Nov 10, 2016

@benfry Ohh, sorry about this. I thought I'd fixed this. Will update this PR and open a new one in processing-docs.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Nov 10, 2016

Member

Great, thanks!

Member

benfry commented Nov 10, 2016

Great, thanks!

@suheb

This comment has been minimized.

Show comment
Hide comment
@suheb

suheb Nov 10, 2016

Contributor

Done. Here's the other PR. Let me know if anything's needed.

Contributor

suheb commented Nov 10, 2016

Done. Here's the other PR. Let me know if anything's needed.

@benfry benfry merged commit a2d413c into processing:master Nov 10, 2016

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Nov 10, 2016

Member

Thanks, I'll let the docs folks handle the other one so that they can run the necessary build scripts.

Member

benfry commented Nov 10, 2016

Thanks, I'll let the docs folks handle the other one so that they can run the necessary build scripts.

benfry added a commit that referenced this pull request Nov 10, 2016

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