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

feat(box-connector): Box Connector #3753

Merged
merged 3 commits into from
Jan 7, 2025
Merged

feat(box-connector): Box Connector #3753

merged 3 commits into from
Jan 7, 2025

Conversation

sbuettner
Copy link
Contributor

@sbuettner sbuettner commented Dec 5, 2024

Description

Add Box.com Connector

Related issues

https://github.com/camunda/team-connectors/issues/958

Related epic

https://github.com/camunda/product-hub/issues/2399

Checklist

  • PR has a milestone or the no milestone label.

@sbuettner sbuettner added this to the 8.7.0-alpha3 milestone Dec 5, 2024
@sbuettner sbuettner changed the title fix(box-connector): Add first version of the Box Connector feat(box-connector): Box Connector Dec 5, 2024
@github-actions github-actions bot temporarily deployed to connectors-958-box-connect-c8sm December 24, 2024 11:33 Destroyed
@sbuettner sbuettner marked this pull request as ready for review December 24, 2024 11:33
@sbuettner sbuettner requested a review from a team as a code owner December 24, 2024 11:33
@github-actions github-actions bot temporarily deployed to connectors-958-box-connect-c8sm December 24, 2024 11:34 Destroyed
@sbuettner sbuettner requested review from mathias-vandaele and johnBgood and removed request for johnBgood December 24, 2024 11:35
@github-actions github-actions bot temporarily deployed to connectors-958-box-connect-c8sm December 24, 2024 12:06 Destroyed
@github-actions github-actions bot temporarily deployed to connectors-958-box-connect-c8sm December 24, 2024 12:52 Destroyed
@github-actions github-actions bot temporarily deployed to connectors-958-box-connect-c8sm December 24, 2024 14:47 Destroyed
@github-actions github-actions bot temporarily deployed to connectors-958-box-connect-c8sm December 24, 2024 14:50 Destroyed
Copy link
Contributor

@mathias-vandaele mathias-vandaele left a comment

Choose a reason for hiding this comment

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

Overall good, few comments.
We could also make more tests, but might be irrelevant, as it would be testing the box api

defaultValue = "developerToken",
description =
"Specify authentication strategy. Learn more at the <a href=\"https://developer.box.com/guides/authentication/\" target=\"_blank\">documentation page</a>")
public sealed interface Authentication
Copy link
Contributor

Choose a reason for hiding this comment

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

Authentication interface could be in another file for readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to have the whole request in a single file. Maybe a matter of taste

Copy link
Collaborator

Choose a reason for hiding this comment

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

I tend to agree with @mathias-vandaele here, I feel it helps when navigating code

name = "type",
defaultValue = "createFolder",
description = "The operation to execute.")
public sealed interface Operation
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, it could be separated for readability

@sbuettner sbuettner added this pull request to the merge queue Jan 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 6, 2025
@sbuettner sbuettner added this pull request to the merge queue Jan 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 6, 2025
@sbuettner sbuettner added this pull request to the merge queue Jan 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 7, 2025
@sbuettner sbuettner added this pull request to the merge queue Jan 7, 2025
@sbuettner sbuettner removed this pull request from the merge queue due to a manual request Jan 7, 2025
@sbuettner sbuettner added this pull request to the merge queue Jan 7, 2025
@johnBgood johnBgood removed this pull request from the merge queue due to a manual request Jan 7, 2025
@johnBgood johnBgood added this pull request to the merge queue Jan 7, 2025
Merged via the queue into main with commit 9653966 Jan 7, 2025
23 of 24 checks passed
@johnBgood johnBgood deleted the 958-box-connector branch January 7, 2025 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants