-
Notifications
You must be signed in to change notification settings - Fork 908
Add script to automatically set up a new module #6118
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
base: master
Are you sure you want to change the base?
Conversation
scripts/setup-new-module
Outdated
|
||
# Check if module already exists | ||
if grep -q "<module>$module_path</module>" "$root_pom"; then | ||
echo "Module already exists in root pom.xml. Skipping." |
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 would this happen and is this something that should be an error?
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.
It could happen when the user has already set up some files manually because they may not know this script exist when they created the new module
scripts/setup-new-module
Outdated
fi | ||
|
||
echo "" | ||
echo "Module setup complete! Please review the changes and complete any manual steps mentioned above." |
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.
I'm not sure how much output this produces, but it might be good to collect the required manual steps and provide them all together here so they are less likely to be missed. (feel free to ignore if its never much output).
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.
There's actually no manual steps anymore :) Will update 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.
Nice! Python version is much easier to read (at least for me) :)
print(f"Using SDK version: {sdk_version}") | ||
|
||
with open(pom_path, 'w') as f: | ||
f.write(f'''<?xml version="1.0" encoding="UTF-8"?> |
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.
Nit: put the start of text on new line for readability and consistency with other text below
|
Motivation and Context
Add script to automatically set up a new module based on the checklist.