-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add ticket custom field number #42
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
pkg/connector/tickets.go (1)
89-97
: LGTM! Consider a minor optimization.The new case for handling
v2.TicketCustomField_NumberValue
is implemented correctly and consistently with other cases. It properly checks for nil values at each step, ensuring robustness.Consider simplifying the nil checks:
case *v2.TicketCustomField_NumberValue: - if v.NumberValue == nil { + if v.NumberValue == nil || v.NumberValue.GetValue() == nil { return nil, nil } - numValue := v.NumberValue.GetValue() - if numValue == nil { - return nil, nil - } - return numValue.GetValue(), nil + return v.NumberValue.GetValue().GetValue(), nilThis change reduces the number of lines while maintaining the same functionality and nil-safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (12)
go.sum
is excluded by!**/*.sum
and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/ticket.pb.go
is excluded by!**/*.pb.go
,!vendor/**
and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/ticket.pb.validate.go
is excluded by!vendor/**
and included by nonevendor/github.com/conductorone/baton-sdk/pkg/cli/commands.go
is excluded by!vendor/**
and included by nonevendor/github.com/conductorone/baton-sdk/pkg/cli/service_windows.go
is excluded by!vendor/**
and included by nonevendor/github.com/conductorone/baton-sdk/pkg/connectorbuilder/connectorbuilder.go
is excluded by!vendor/**
and included by nonevendor/github.com/conductorone/baton-sdk/pkg/sdk/version.go
is excluded by!vendor/**
and included by nonevendor/github.com/conductorone/baton-sdk/pkg/types/ticket/custom_fields.go
is excluded by!vendor/**
and included by nonevendor/github.com/conductorone/baton-sdk/pkg/uhttp/gocache.go
is excluded by!vendor/**
and included by nonevendor/github.com/conductorone/baton-sdk/pkg/uhttp/wrapper.go
is excluded by!vendor/**
and included by nonevendor/google.golang.org/protobuf/types/known/wrapperspb/wrappers.pb.go
is excluded by!**/*.pb.go
,!vendor/**
and included by nonevendor/modules.txt
is excluded by!vendor/**
and included by none
📒 Files selected for processing (2)
- go.mod (1 hunks)
- pkg/connector/tickets.go (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
go.mod (1)
6-6
: Verify the intentionality and impact of the SDK update.The
github.com/conductorone/baton-sdk
dependency has been updated from v0.2.33 to v0.2.35. This minor version update likely includes new features or non-breaking changes.
- Can you confirm that this update is intentional and related to the PR objective of adding a ticket custom field number?
- Have you verified compatibility with the new SDK version, especially concerning the changes made in the
pkg/connector/tickets.go
file?To ensure there are no unintended consequences, please run the following command to check for any breaking changes or deprecations in the SDK update:
This will help identify any potential issues that may arise from the SDK update.
✅ Verification successful
Attempting to fetch and analyze the release notes again for
github.com/conductorone/baton-sdk
version v0.2.35.
SDK update verified and poses no breaking changes or deprecations.
The
github.com/conductorone/baton-sdk
dependency update to v0.2.35 is intentional and aligns with the PR objectives.No breaking changes or deprecations were found in the release notes or the
pkg
directory, ensuring compatibility with the new SDK version.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes or deprecations in the SDK update # Fetch the changelog or release notes for the baton-sdk gh repo view conductorone/baton-sdk --json url -q .url | xargs -I {} gh api {}/releases | jq -r '.[] | select(.tag_name == "v0.2.35") | .body' # Search for any breaking changes or deprecation notices rg -i "breaking change|deprecat" pkgLength of output: 280
Script:
#!/bin/bash # Description: Fetch the release notes for baton-sdk v0.2.35 and check for breaking changes or deprecations # Fetch the release notes for tag v0.2.35 gh api repos/conductorone/baton-sdk/releases/tags/v0.2.35 --jq .body # Search for any breaking changes or deprecation notices in the release notes gh api repos/conductorone/baton-sdk/releases/tags/v0.2.35 --jq .body | rg -i "breaking change|deprecat" # Additionally, search the pkg directory for breaking changes or deprecation notices rg -i "breaking change|deprecat" pkgLength of output: 436
pkg/connector/tickets.go (1)
89-97
: Verify usage of updatedcustomFieldSchemaToMetaField
method.The addition of number value handling enhances the method's capabilities. To ensure this change is fully utilized:
- Verify that all callers of
customFieldSchemaToMetaField
are aware of the new number value support.- Check if any existing number fields were previously handled as strings and update them if necessary.
Run the following script to find usages of
customFieldSchemaToMetaField
:Review the results to ensure proper utilization of the new number value support.
✅ Verification successful
Verification Successful: No issues found
The
customFieldSchemaToMetaField
method is properly utilized withinpkg/connector/tickets.go
, and number fields are correctly handled. No further action is required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find usages of customFieldSchemaToMetaField and potential number fields handled as strings # Search for method calls echo "Usages of customFieldSchemaToMetaField:" rg --type go "customFieldSchemaToMetaField\(" -A 5 # Search for potential number fields handled as strings echo "\nPotential number fields handled as strings:" rg --type go "case jira\.TypeNumber:"Length of output: 1061
Pull Request
To help us get this pull request reviewed and merged quickly, please be sure to include the following items:
PR Type
What kind of change does this PR introduce?
Backward Compatibility
Is this change backward compatible with the most recently released version? Does it introduce changes which might change the user experience in any way? Does it alter the Connector in any way?
Useful links:
Issue Linking
What's new?
Summary by CodeRabbit
baton-sdk
dependency to improve functionality and compatibility.