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

refactor(idutils): rename GenerateExternalIDForDocument to GenerateExternalID #1713

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tomazpu
Copy link
Contributor

@tomazpu tomazpu commented Feb 19, 2025

What this PR does / Why we need it:

As the GenerateExternalIDForDocument is being used also in other API than Documents, it was renamed to GenerateExternalID

Special notes for your reviewer:

Does this PR introduce a user-facing change?

@tomazpu tomazpu requested a review from a team as a code owner February 19, 2025 07:47
@tomazpu tomazpu self-assigned this Feb 19, 2025
Copy link

Unit Test Results

1 812 tests  ±0   1 811 ✅ ±0   53s ⏱️ ±0s
  132 suites ±0       1 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 8f5c867. ± Comparison against base commit 4f045c2.

@tomazpu tomazpu marked this pull request as draft February 19, 2025 07:50
@@ -51,8 +52,8 @@ func GenerateExternalIDForSettingsObject(c coordinate.Coordinate) (string, error

type ExternalIDGenerator func(coordinate.Coordinate) (string, error)

// GenerateExternalIDForDocument generates an external ID for a document configuration. It is under 50 characters long and uses at most only "a-z", "A-Z", "0-9" and "-".
func GenerateExternalIDForDocument(c coordinate.Coordinate) (string, error) {
// GenerateExternalID generates an external ID for a document configuration. It is under 50 characters long and uses at most only "a-z", "A-Z", "0-9" and "-".
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
// GenerateExternalID generates an external ID for a document configuration. It is under 50 characters long and uses at most only "a-z", "A-Z", "0-9" and "-".
// GenerateExternalID generates an external ID for a configuration. It is under 50 characters long and uses at most only "a-z", "A-Z", "0-9" and "-".

@@ -51,8 +52,8 @@ func GenerateExternalIDForSettingsObject(c coordinate.Coordinate) (string, error

type ExternalIDGenerator func(coordinate.Coordinate) (string, error)

// GenerateExternalIDForDocument generates an external ID for a document configuration. It is under 50 characters long and uses at most only "a-z", "A-Z", "0-9" and "-".
func GenerateExternalIDForDocument(c coordinate.Coordinate) (string, error) {
// GenerateExternalID generates an external ID for a document configuration. It is under 50 characters long and uses at most only "a-z", "A-Z", "0-9" and "-".
Copy link
Contributor

Choose a reason for hiding this comment

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

Just an idea: As the function takes the coordinate, could it absorb the function above for settings and then provide the correct externalID after examining the config type?

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 like the idea, lets align at the daily :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Kind of like that as well, though we do increase cohesion 🤔

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.

3 participants