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

[C++] Migrate JSON Integration code to Result<> #37567

Closed
pitrou opened this issue Sep 5, 2023 · 0 comments · Fixed by #37573
Closed

[C++] Migrate JSON Integration code to Result<> #37567

pitrou opened this issue Sep 5, 2023 · 0 comments · Fixed by #37573

Comments

@pitrou
Copy link
Member

pitrou commented Sep 5, 2023

Describe the enhancement requested

The JSON Integration code is not a public API, so this was overlooked. Nevertheless, migrating to Result-returning APIs will make it easier to use for us.

Component(s)

C++

@pitrou pitrou self-assigned this Sep 5, 2023
pitrou added a commit to pitrou/arrow that referenced this issue Sep 5, 2023
pitrou added a commit that referenced this issue Sep 5, 2023
### Rationale for this change

The JSON Integration APIs still use the old-style of returning `Status` together with a `T*` out-parameter.
While those are internal APIs, it would still improve ease of us to migrate them to the newer idiom of returning a `Result<T>`.

### What changes are included in this PR?

Migrate the relevant APIs from `Status` to `Result`. Since the APIs are internal, no deprecation period is introduced.

Also, some very minor style and testing cleanups here and there.

### Are these changes tested?

By existing tests.

### Are there any user-facing changes?

No.

* Closes: #37567

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 14.0.0 milestone Sep 5, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…he#37573)

### Rationale for this change

The JSON Integration APIs still use the old-style of returning `Status` together with a `T*` out-parameter.
While those are internal APIs, it would still improve ease of us to migrate them to the newer idiom of returning a `Result<T>`.

### What changes are included in this PR?

Migrate the relevant APIs from `Status` to `Result`. Since the APIs are internal, no deprecation period is introduced.

Also, some very minor style and testing cleanups here and there.

### Are these changes tested?

By existing tests.

### Are there any user-facing changes?

No.

* Closes: apache#37567

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…he#37573)

### Rationale for this change

The JSON Integration APIs still use the old-style of returning `Status` together with a `T*` out-parameter.
While those are internal APIs, it would still improve ease of us to migrate them to the newer idiom of returning a `Result<T>`.

### What changes are included in this PR?

Migrate the relevant APIs from `Status` to `Result`. Since the APIs are internal, no deprecation period is introduced.

Also, some very minor style and testing cleanups here and there.

### Are these changes tested?

By existing tests.

### Are there any user-facing changes?

No.

* Closes: apache#37567

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant