Skip to content

Conversation

@tuteng
Copy link
Member

@tuteng tuteng commented May 25, 2019

Motivation

The current rest api document parameters are missing descriptions and some parameters are missing.

Modifications

Improve document of topics

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@jiazhai
Copy link
Member

jiazhai commented May 25, 2019

rerun java8 tests
for

org.apache.pulsar.functions.worker.PulsarFunctionStateTest.setup

@jiazhai jiazhai added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. component/website type/bug The PR fixed a bug or issue reported a bug labels May 25, 2019
@tuteng
Copy link
Member Author

tuteng commented May 25, 2019

@jennifer88huang @Anonymitaet Can you take the time to help review the descriptive information of these modifications? Thank you very much

Copy link
Contributor

@Jennifer88huang-zz Jennifer88huang-zz left a comment

Choose a reason for hiding this comment

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

Good job, I just refine some typos. If you refine one of them, remember to check other same cases in this file.

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

@tuteng Can you please check each method and make sure all the response codes are documented?

e.g. 500 should be documented almost for each method (but you need to double check the code).

I made a few comments on the first few methods. Please use them as examples to update other methods.

@tuteng
Copy link
Member Author

tuteng commented May 25, 2019

@tuteng Can you please check each method and make sure all the response codes are documented?

e.g. 500 should be documented almost for each method (but you need to double check the code).

I made a few comments on the first few methods. Please use them as examples to update other methods.

I'll check it later.

@tuteng
Copy link
Member Author

tuteng commented Jun 6, 2019

@jennifer88huang @Anonymitaet @sijie PTAL

Copy link
Contributor

@Jennifer88huang-zz Jennifer88huang-zz left a comment

Choose a reason for hiding this comment

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

When you update one place, make sure you update other similar cases in the file.

@sijie
Copy link
Member

sijie commented Jun 9, 2019

@tuteng can you address the latest review comments?

# Conflicts:
#	pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
@tuteng tuteng force-pushed the fix/topics-document-improve branch from 93e1fe9 to f639675 Compare June 9, 2019 12:08
@tuteng
Copy link
Member Author

tuteng commented Jun 9, 2019

Done

@tuteng
Copy link
Member Author

tuteng commented Jun 10, 2019

rerun java8 tests

1 similar comment
@tuteng
Copy link
Member Author

tuteng commented Jun 11, 2019

rerun java8 tests

@merlimat merlimat merged commit a195221 into apache:master Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants