-
Notifications
You must be signed in to change notification settings - Fork 38
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(sdk-schematics): initialize git repository #1361
base: main
Are you sure you want to change the base?
Conversation
b3ef644
to
ebf0564
Compare
@@ -28,6 +28,11 @@ | |||
"description": "Skip NPM install", | |||
"default": false | |||
}, | |||
"skipGit": { |
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 should also add the commit
option
You can find an example in this file packages/@o3r/workspace/schematics/ng-add/schema.json
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.
Thank you @matthieu-crouzet for your comment! I have added the commit
option in the shell generator schematics. However, it is currently not used anywhere.
I have some questions:
- Should the
commit
option of the shell generator schematics betrue
by default? - Should the
npm create @ama-sdk
command create a commit? - Should the
npm create @ama-sdk
command use thecommit
option of the schematics to create a commit? - If the answer to the previous question is yes, the commit would only contain the shell files and neither the result of
yarn set version
nor the output of sdk generation... so for those extra changes, we could either create another commit or amend the previous one?
I personally prefer not creating a commit in npm create
(as it is done in the current state of this PR), but if it is decided to create one, then I think it would be better to create it directly with the right files (calling the shell generator schematics with the commit
option set to false
and creating the commit at the end), rather than amending a commit. Note that if a repository already exists in a parent folder, the RepositoryInitializerTask
does nothing, so we should be especially careful not to amend a previous commit in this case.
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.
Indeed it should not be part of the shell
schematic but more of the create
script at the end with all the files.
It's the behavior that we have by default when run npm create @o3r
if I'm not mistaken.
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.
npm create @o3r
creates a commit, but does not include all changes in it.
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.
In your opinion should we create an issue to handle it later
I think it could be great if the repo is "clean" when you finish the creation
5066f28
to
9651d8b
Compare
9651d8b
to
d43edab
Compare
d43edab
to
8d7efee
Compare
9edac99
to
b855e10
Compare
b855e10
to
5890cf2
Compare
Proposed change
Initialize a git repository when using either
npm create @ama-sdk
or the sdk schematics.