Re: [PR] Fix coder encode decode javadoc [beam]
Subramanya-Veeregowda closed pull request #37963: Fix coder encode decode javadoc URL: https://github.com/apache/beam/pull/37963 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
stankiewicz commented on code in PR #37963:
URL: https://github.com/apache/beam/pull/37963#discussion_r3101619462
##
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##
@@ -37,7 +37,9 @@
import org.checkerframework.checker.nullness.qual.Nullable;
/**
- * A {@link Coder Coder} defines how to encode and decode values of
type {@code T} into
+ * Use {@link CoderProperties} to verify the correctness of a {@link Coder}
implementation.
Review Comment:
does have to be first sentence?
##
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##
Review Comment:
there a lot of duplicate javadoc, please simplify it a bit.
##
.gitignore:
##
Review Comment:
shouldn't be part of commit
##
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##
@@ -60,6 +74,14 @@ public abstract class Coder implements Serializable {
* @deprecated To implement a coder, do not use any {@link Context}. Just
implement only those
* abstract methods which do not accept a {@link Context} and leave the
default
* implementations for methods accepting a {@link Context}.
+ * There are two common contexts:
Review Comment:
why "common"?
##
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##
@@ -52,6 +54,18 @@
* All methods of a {@link Coder} are required to be thread safe.
*
* @param the type of values being encoded and decoded
+ *
+ * When implementing a {@link Coder}, it is important to understand the role
Review Comment:
Does it have to be part of this javadoc?
##
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##
@@ -117,8 +139,29 @@ public String toString() {
* know how many bytes to read when decoding. A common approach is to prefix
the encoding with the
* element's encoded length.
*
+ * The encoding behavior depends on the {@link Context}:
+ *
+ *
+ * In {@link Context#OUTER}, the value occupies the remainder of the
stream, so no
Review Comment:
those descriptions are inconsistent.
##
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##
@@ -117,8 +139,29 @@ public String toString() {
* know how many bytes to read when decoding. A common approach is to prefix
the encoding with the
* element's encoded length.
*
+ * The encoding behavior depends on the {@link Context}:
+ *
+ *
+ * In {@link Context#OUTER}, the value occupies the remainder of the
stream, so no
+ * additional length information is required.
+ * In {@link Context#NESTED}, the value is part of a larger structure,
so the encoding must
+ * include sufficient information (such as length prefixes) to allow
correct decoding of
+ * subsequent values.
+ *
+ *
+ * Implementations must ensure that encoded values are self-delimiting
when used in a nested
+ * context.
+ *
* @throws IOException if writing to the {@code OutputStream} fails for some
reason
* @throws CoderException if the value could not be encoded for some reason
+ *
+ * Implementers must ensure that encoding in {@link Context#NESTED}
Review Comment:
this is duplicate but written slightly differently.
##
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##
@@ -52,6 +54,18 @@
* All methods of a {@link Coder} are required to be thread safe.
*
* @param the type of values being encoded and decoded
+ *
+ * When implementing a {@link Coder}, it is important to understand the role
+ * of the encoding {@link Context}. The context determines whether the encoded
+ * value must be self-delimiting or can consume the remainder of the stream.
+ *
+ * Implementations should ensure that values encoded in {@link
Context#NESTED}
+ * include sufficient length information (such as prefixes) to allow correct
+ * decoding of subsequent values.
+ *
+ * Use {@link CoderProperties} in tests to verify correctness, determinism,
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
stankiewicz commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4269390136 don't commit gitignore -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
stankiewicz commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4268163552 waiting on Author -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
github-actions[bot] commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4268016229 Reminder, please take a look at this pr: @kennknowles -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
stankiewicz commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4216613605 > @stankiewicz Can you tell me why my checks are failing and what is the issue, I am new into this world so i can't figure it out on my own if there are any specific needs for improvements i would like to work on it start with PreCommit Spotless - https://github.com/apache/beam/actions/runs/24007159669/job/70012618083?pr=37963 in logs it states which file has a problem and how to fix code formatting. Before submitting PR, it's worth setting up environment - following guides can help you: - https://github.com/apache/beam/blob/master/CONTRIBUTING.md - https://cwiki.apache.org/confluence/display/BEAM/Java+Tips - https://cwiki.apache.org/confluence/display/BEAM/Using+IntelliJ+IDE There are also agent skills added that will help you finding some gradle tasks, like spotless. works really well with gemini cli and antigravity. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
codecov[bot] commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4189436298 ## [Codecov](https://app.codecov.io/gh/apache/beam/pull/37963?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report :white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 56.84%. Comparing base ([`97c2faf`](https://app.codecov.io/gh/apache/beam/commit/97c2faf919daab04344e36a9dbd16502305239ca?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`25af774`](https://app.codecov.io/gh/apache/beam/commit/25af774d8c3875a4c9e640e11d022a41236eb8b1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)). :warning: Report is 82 commits behind head on master. Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #37963 +/- ## - Coverage 57.34% 56.84% -0.50% + Complexity 5115 3426-1689 Files 1392 1178 -214 Lines197103 187943-9160 Branches 4817 3593-1224 - Hits 113020 106841-6179 + Misses8024777710-2537 + Partials 3836 3392 -444 ``` | [Flag](https://app.codecov.io/gh/apache/beam/pull/37963/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [java](https://app.codecov.io/gh/apache/beam/pull/37963/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.94% <ø> (+1.05%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/beam/pull/37963?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :rocket: New features to boost your workflow: - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
Subramanya-Veeregowda commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4174483537 a local issue where i added javadoc may be the file is not added, Sorry for the empty PR, i will update it ASAP! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
stankiewicz commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4172855388 @Subramanya-Veeregowda pr is empty, should we close it or are you planning to continue working on this? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
github-actions[bot] commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4160369955 Assigning reviewers: R: @kennknowles for label java. Note: If you would like to opt out of this review, comment `assign to next reviewer`. Available commands: - `stop reviewer notifications` - opt out of the automated review tooling - `remind me after tests pass` - tag the comment author after tests pass - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers) The PR bot will only process comments in the main thread (not review comments). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
Subramanya-Veeregowda commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4153718154 Why checks are stuck should I draft and recommit again? Any further improvements needed? Open to improve if any specific improvements needed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
Subramanya-Veeregowda commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4148125442 Re-running CI, it looks like the failure is in macOS tests and may be unrelated to the Javadoc changes. Could you please confirm if this is a flaky issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
github-actions[bot] commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4137282228 Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix coder encode decode javadoc [beam]
gemini-code-assist[bot] commented on PR #37963: URL: https://github.com/apache/beam/pull/37963#issuecomment-4135042979 ## Summary of Changes Hello, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on enhancing documentation across two key areas: improving the Javadoc for the `Coder` class in Java to clarify the usage and purpose of `Context` in encoding and decoding operations, and expanding the Python programming guide with new examples for advanced field selection patterns. The changes aim to provide clearer guidance and better understanding for developers using these functionalities. ### Highlights * **Improved Javadoc for Coder.Context**: The Javadoc for the `Coder.Context` class has been significantly enhanced, including a proper summary line, clearer explanations of `OUTER` vs `NESTED` contexts, and practical usage examples. Redundant Javadoc for individual fields within `Context` was removed as the class-level documentation now covers their meaning. * **Enhanced Coder `encode` and `decode` Javadoc**: The Javadoc for both the `encode` and `decode` methods in the `Coder` class has been updated. This includes better descriptions of how values are handled, the importance of boundary information for multiple elements, and how `Context` influences encoding/decoding. The `@deprecated` messages for context-aware methods were also refined. * **Added Python SDK documentation examples**: New examples have been added to the programming guide for the Python SDK, demonstrating how to use wildcard selection for nested fields, select nested fields within arrays, and select specific keys from map fields. 🧠 **New Feature in Public Preview:** You can now enable **Memory** to help **Gemini Code Assist** learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. **Click [here](https://codeassist.google/code-review/login) to enable Memory in your admin console.** Using Gemini Code Assist The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either `/gemini ` or `@gemini-code-assist `. Below is a summary of the supported commands on the current page. Feature | Command | Description --- | --- | --- Code Review | `/gemini review` | Performs a code review for the current pull request in its current state. Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state. Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. Help | `/gemini help` | Displays a list of available commands. Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG). [^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
