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

[ENHANCEMENT] percli dac setup adds output dir to .gitignore #1710

Merged

Conversation

AntoineThebaud
Copy link
Contributor

@AntoineThebaud AntoineThebaud commented Jan 16, 2024

Description

Just a simple addition to avoid further manual steps to the DaC users.
This step is skipped if the .gitignore file is not already present.

Checklist

  • Pull request has a descriptive title and context useful to a reviewer.
  • Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the
    following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, DOC,IGNORE.
  • All commits have DCO signoffs.

@AntoineThebaud AntoineThebaud changed the base branch from main to antoinethebaud/DaC-build-command January 16, 2024 21:52
@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/DaC-build-command branch 3 times, most recently from 89987eb to 447e6ae Compare January 19, 2024 09:31
Base automatically changed from antoinethebaud/DaC-build-command to feat/dashboard-as-code January 19, 2024 09:45
@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/DaC-create-gitignore branch 3 times, most recently from 72e8bd5 to 0b16851 Compare January 19, 2024 10:10
@AntoineThebaud AntoineThebaud marked this pull request as ready for review January 19, 2024 16:20
@AntoineThebaud
Copy link
Contributor Author

'should make this idempotent by adding a check to avoid appending to gitignore if the built folder is already listed.

@AntoineThebaud AntoineThebaud marked this pull request as draft January 20, 2024 08:45
@AntoineThebaud AntoineThebaud marked this pull request as ready for review January 22, 2024 14:22
Copy link
Member

@celian-garcia celian-garcia left a comment

Choose a reason for hiding this comment

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

If you want my opinion, I'm not fan of pre-assuming that the user of your tool want to create a git repo, or want to use git as a version manager or even want to ignore the built folder.

That's easier to have a doc saying.

git init
percli dac setup
echo "built" >> .gitignore

Less code to maintain and the three lines are self explanatory.

@Nexucis
Copy link
Member

Nexucis commented Jan 22, 2024

I would be in the middle here:

  • In case .gitignore exists, then we could append it with the folder built and I think we should add a comment in the .gitignore explaining what is this folder ignored. Like
# folder used to store the result of the command percli dac build
/built
  • In other case, I think we shouldn't assume the user is using git or want to use it and so for .gitignore. A user could even have a global .gitignore and so it doesn't need to create one for each project.

  • Regarding the documentation, we should just explain what the command is doing. There is no requirement when running the command percli dac setup to use git or to init a repo with git.

Finally, it would be great to be able to change the targeted folder too.

@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/DaC-create-gitignore branch from 7b8e8e5 to aee1939 Compare January 23, 2024 08:55
@AntoineThebaud AntoineThebaud changed the title [ENHANCEMENT] add .gitignore generation to percli dac setup [ENHANCEMENT] percli dac setup adds output dir to .gitignore Jan 23, 2024
@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/DaC-create-gitignore branch from aee1939 to ba95ff7 Compare January 23, 2024 08:58
@AntoineThebaud AntoineThebaud marked this pull request as draft February 11, 2024 18:53
@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/DaC-create-gitignore branch from 2ddcff9 to cdfd1ab Compare February 11, 2024 19:05
@AntoineThebaud AntoineThebaud changed the base branch from feat/dashboard-as-code to antoinethebaud/dac-custom-output-folder February 11, 2024 19:05
@AntoineThebaud AntoineThebaud marked this pull request as ready for review February 11, 2024 19:20
@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/dac-custom-output-folder branch from 1ae0d7b to 7e23bb8 Compare February 12, 2024 14:46
Base automatically changed from antoinethebaud/dac-custom-output-folder to feat/dashboard-as-code February 13, 2024 09:00
@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/DaC-create-gitignore branch 2 times, most recently from 1b16792 to ea8cb96 Compare February 13, 2024 09:17
Signed-off-by: Antoine THEBAUD <antoine.thebaud@yahoo.fr>
@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/DaC-create-gitignore branch from ea8cb96 to a8914b6 Compare February 13, 2024 09:45
@AntoineThebaud AntoineThebaud merged commit 7b7fda5 into feat/dashboard-as-code Feb 13, 2024
19 checks passed
@AntoineThebaud AntoineThebaud deleted the antoinethebaud/DaC-create-gitignore branch February 13, 2024 10:12
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.

None yet

3 participants