-
Notifications
You must be signed in to change notification settings - Fork 10
breaking: support use of codacy api token - PLUTO-1379 #58
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
Conversation
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.
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
.codacy/codacy.yaml:5
- Verify that downgrading ESLint from 9.3.0 to 8.57.0 is intentional, as this may introduce compatibility issues if other parts of the project expect the newer version.
cmd/init.go:224
- Consider including the HTTP status code or response details in the error message to improve debugging when the Codacy API call fails.
return errors.New("failed to get repository's configuration from Codacy API")
0d3187b to
9527ac8
Compare
9527ac8 to
b8b9eae
Compare
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.
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- .codacy/cli-config.yaml: Language not supported
- .codacy/codacy.yaml: Language not supported
|
|
||
| for _, tool := range tools { | ||
| if tool.Uuid == "f8b29663-2cb2-498d-b923-a10c6a8c05cd" { | ||
| switch tool.Uuid { |
Copilot
AI
Apr 7, 2025
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.
The switch on tool.Uuid compares against string(ESLint), but ESLint is defined as 'eslint' in domain/toolName.go, which is not a UUID. Use the proper UUID constants (e.g. those defined as ToolUiid) for accurate comparisons.
cmd/init.go
Outdated
| case string(ESLint): | ||
| if len(patternConfiguration) > 0 { |
Copilot
AI
Apr 7, 2025
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.
This case statement compares tool.Uuid to string(ESLint) where the constant value 'eslint' does not match the expected UUID. Replace it with the correct UUID constant from ToolUiid to ensure proper matching.
| case string(ESLint): | |
| if len(patternConfiguration) > 0 { | |
| case tools.ToolUuidESLint: |
cmd/init.go
Outdated
|
|
||
| func init() { | ||
| initCmd.Flags().StringVar(&codacyRepositoryToken, "repository-token", "", "optional codacy repository token, if defined configurations will be fetched from codacy") | ||
| initCmd.Flags().StringVar(&codacyApiToken, "codacy-api-token", "", "optional codacy api token, if defined configurations will be fetched from codacy") |
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.
We can simplify it
| initCmd.Flags().StringVar(&codacyApiToken, "codacy-api-token", "", "optional codacy api token, if defined configurations will be fetched from codacy") | |
| initCmd.Flags().StringVar(&codacyApiToken, "api-token", "", "optional codacy api token, if defined configurations will be fetched from codacy") |
cmd/init.go
Outdated
| func init() { | ||
| initCmd.Flags().StringVar(&codacyRepositoryToken, "repository-token", "", "optional codacy repository token, if defined configurations will be fetched from codacy") | ||
| initCmd.Flags().StringVar(&codacyApiToken, "codacy-api-token", "", "optional codacy api token, if defined configurations will be fetched from codacy") | ||
| initCmd.Flags().StringVar(&remoteProvider, "provider", "", "optional provider (gh/bb/gl), if defined configurations will be fetched from codacy") |
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.
We can focus on a semantic of 'mandatory when the api-token is defined. Will help users and the model
cmd/init.go
Outdated
|
|
||
| // Create a new GET request | ||
| req, err := http.NewRequest("GET", url, nil) | ||
| apiTools, err := tools.GetRepositoryTools(CodacyApiBase, token, remoteProvider, remoteOrganizationName, remoteRepositoryName) |
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.
Can we make it something like repositoryTools. We aligned that GetRepositoryTools returns the tools for the repository in question (the ones that we are interested, meaning the enabled ones), already filtered by what is supported on the CLI.
//Concepts:
repositoryTools + supportedToolsOnCLI => configuredRepositoryTools
//supportedToolsOnCLI
Come from the tools on the configuration .tools (config is available globally, but in some places we also pass it)
cmd/init.go
Outdated
| remoteRepositoryName, | ||
| tool.Uuid) | ||
|
|
||
| fmt.Printf("url: %q\n", url) |
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.
Probably debug and remove?
cmd/init.go
Outdated
| }, | ||
| ) | ||
| createToolFileConfigurations(tool, apiToolConfigurations) | ||
| // TODO: Process the response and create configuration files for each tool |
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.
leftovers delete
cmd/init.go
Outdated
|
|
||
| //ESLInt internal codacy uuid, to filter ot not ESLint tools | ||
| //"f8b29663-2cb2-498d-b923-a10c6a8c05cd" | ||
| eslintConfigFile, err := os.Create("eslint.config.mjs") |
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.
When rebasing the @andrzej-janczak PR (already merged) make sure you don't break the new path to create those config files
429f844 to
b908087
Compare
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
b908087 to
b3932f5
Compare
a015dce to
b4bfe46
Compare
Removed support for project-token (repository token).