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: stop requiring gitlab_project_id for gitlab templates #941

Merged
merged 3 commits into from
Aug 27, 2024

Conversation

TomerHeber
Copy link
Collaborator

@TomerHeber TomerHeber commented Aug 25, 2024

Issue & Steps to Reproduce / Feature Request

fixes #940

Solution

  1. Deprecate the field in all relevant schemas.
  2. Updated code.
  3. Updated acceptance and harness tests.
  4. Updated structs.

default:
return fmt.Errorf("unhandled type %s", putPayload.Type)
}

vcsCounter := 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed this because this may cause some issues. Not clear how to resolve.
Schema validation is a nice to have. Instead BE will error on this.

@@ -711,45 +707,6 @@ func TestUnitEnvironmentDiscoveryConfigurationResource(t *testing.T) {
runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {})
})

t.Run("error: no vcs set", func(t *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed relevant tests.

Copy link
Contributor

@yaronya yaronya left a comment

Choose a reason for hiding this comment

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

Approved, but please change the text

@@ -143,10 +142,10 @@ func resourceEnvironmentDiscoveryConfiguration() *schema.Resource {
Optional: true,
},
"gitlab_project_id": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

@@ -152,14 +153,13 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
Type: schema.TypeString,
Description: "the git token id to be used",
Optional: true,
ConflictsWith: allVCSAttributesBut("token_id", "gitlab_project_id", "is_azure_devops", "path"),
ConflictsWith: allVCSAttributesBut("token_id", "is_azure_devops", "path"),
},
"gitlab_project_id": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

RequiredWith: []string{"token_id"},
Type: schema.TypeInt,
Description: "the project id of the relevant repository (deprecated)",
Deprecated: "'repository' is used instead",
Copy link
Contributor

Choose a reason for hiding this comment

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

Text should be changed
We should use "project id is now auto-fetched from the repository URL"

@github-actions github-actions bot added ready to merge PR approved - can be merged once the PR owner is ready and removed pending final review labels Aug 26, 2024
@Wassap124 Wassap124 merged commit 08b9519 into main Aug 27, 2024
4 checks passed
@Wassap124 Wassap124 deleted the feat-remove-gitlab_project_id-#940 branch August 27, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-client blocked feature integration-tests provider ready to merge PR approved - can be merged once the PR owner is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop requiring gitlab_project_id for gitlab templates
3 participants