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

When using esp-idf-kconfig as a pre-commit hook, it should modify Kconfig file instead of creating Kconfig.new (IDFGH-13810) #9

Closed
3 tasks done
igrr opened this issue Oct 3, 2024 · 1 comment
Assignees
Labels

Comments

@igrr
Copy link
Member

igrr commented Oct 3, 2024

Checklist

  • Checked the issue tracker for similar issues to ensure this is not a duplicate.
  • Described the feature in detail and justified the reason for the request.
  • Provided specific use cases and examples.

Feature description

Currently when esp-idf-kconfig is used as a pre-commit hook, it seems to create Kconfig.new files with suggested changes.

/path/to/component/Kconfig:7: line should be shorter than 120 characters
/path/to/component/Kconfig:8: line should be shorter than 120 characters
	/path/to/component/Kconfig.new has been saved with suggestions for resolving the issues.
	Please note that the suggestions can be wrong and you might need to re-run the checker several times for solving all issues

Within pre-commit framework, it is more common that the hook modifies the file being checked directly. This makes it easier for the developer to review the suggested change and add it to the commit. If the change is not desired, the developer can easily discard the unstaged change.

An argument such as --replace could be introduced, and passed by default to kconfcheck in .pre-commit-hooks.yaml.

Use cases

Adding esp-idf-kconfig as a pre-commit hook to a repository with several Kconfig files.

Alternatives

Manually do mv path/to/Kconfig.new path/to/Kconfig for each file processed by the pre-commit hook.

Additional context

No response

@igrr igrr added the Type: Feature Request Feature Request for esp-idf-kconfig label Oct 3, 2024
@github-actions github-actions bot changed the title When using esp-idf-kconfig as a pre-commit hook, it should modify Kconfig file instead of creating Kconfig.new When using esp-idf-kconfig as a pre-commit hook, it should modify Kconfig file instead of creating Kconfig.new (IDFGH-13810) Oct 3, 2024
@espressif-bot espressif-bot added Status: Selected for Development Issue is selected for development and removed Status: Opened labels Oct 23, 2024
@espressif-bot espressif-bot added Status: In Progress Issue is being worked on and removed Status: Selected for Development Issue is selected for development labels Oct 30, 2024
@Honza0297
Copy link
Collaborator

Thank you @igrr for the suggestion! I'll prepare a MR.

@espressif-bot espressif-bot added Status: Reviewing and removed Status: In Progress Issue is being worked on labels Oct 30, 2024
espressif-bot pushed a commit that referenced this issue Nov 29, 2024
…hook

When running as pre-commit hook (specified by --replace flag), kconfcheck
will make apply the changes directly to the original file and does not
print message about re-running the check with standalone kconfcheck.

Closes #9
@espressif-bot espressif-bot added Status: Done and removed Status: In Progress Issue is being worked on labels Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants