Skip to content
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

Fixes #10293 - Improve documentation on how to write a response body … #10434

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Aug 29, 2023

…in Jetty 12.

Updated documentation about:

  • Content.Source
  • Content.Sink
  • Handler
  • Request/Response

Updated few APIs to make easier to write applications.

…in Jetty 12.

Updated documentation about:
* Content.Source
* Content.Sink
* Handler
* Request/Response

Updated few APIs to make easier to write applications.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet linked an issue Aug 29, 2023 that may be closed by this pull request
* @return a new {@link Completable} passed to the consumer
* @see #with(Consumer)
*/
public Completable compose(Consumer<Completable> consumer)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a change that's bigger than just documentation.

Fields formFields = FormFields.from(request).get();
return Fields.combine(queryFields, formFields);
CompletableFuture<Fields> contentFields = FormFields.from(request);
return contentFields.thenApply(formFields -> Fields.combine(queryFields, formFields));
Copy link
Contributor

Choose a reason for hiding this comment

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

Another change that isn't just documentation? is this what broke the tests in this latest build?

* @param charset the charset to use to decode bytes
* @return the {@link CompletableFuture} to notify when the whole content has been read
*/
static CompletableFuture<String> asStringAsync(Source source, Charset charset)
Copy link
Contributor

Choose a reason for hiding this comment

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

Another non-documentation change?

Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

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

This PR is bigger than "just documentation".
Some niggles about @Deprecated use.

@@ -25,7 +25,9 @@ public class HttpStatus
public static final int CONTINUE_100 = 100;
public static final int SWITCHING_PROTOCOLS_101 = 101;
public static final int PROCESSING_102 = 102;
@Deprecated(forRemoval = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also have since ...

Suggested change
@Deprecated(forRemoval = true)
@Deprecated(forRemoval = true, since = "12.0.1")

@@ -104,7 +106,9 @@ public enum Code
CONTINUE(CONTINUE_100, "Continue"),
SWITCHING_PROTOCOLS(SWITCHING_PROTOCOLS_101, "Switching Protocols"),
PROCESSING(PROCESSING_102, "Processing"),
@Deprecated(forRemoval = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@Deprecated(forRemoval = true)
@Deprecated(forRemoval = true, since = "12.0.1"

@@ -123,7 +127,7 @@ public enum Code
NOT_MODIFIED(NOT_MODIFIED_304, "Not Modified"),
USE_PROXY(USE_PROXY_305, "Use Proxy"),
TEMPORARY_REDIRECT(TEMPORARY_REDIRECT_307, "Temporary Redirect"),
// Keeping the typo for backward compatibility for a while
@Deprecated(forRemoval = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@Deprecated(forRemoval = true)
@Deprecated(forRemoval = true, since = "12.0.1"

@sbordet sbordet merged commit c638753 into jetty-12.0.x Aug 29, 2023
2 checks passed
@sbordet sbordet deleted the fix/jetty-12-10293-handler-docs branch August 29, 2023 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve documentation on how to write a response body in Jetty 12
2 participants