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

refactor: improve build scripts #23

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

Nelsonochoam
Copy link
Contributor

Description of changes:
Changes the build script so that new template names do not need to be added and get automatically build by the build script.

Why is this change necessary:
Dev don't have to remember about adding the new template folder name to the build script

dgggIoT
dgggIoT previously approved these changes Feb 14, 2023
Comment on lines 1 to 8

python_templates=$(ls -d */ | sed 's/\//\n/g')
current_directory=`pwd`
for template in $maven_templates; do
cd $current_directory/$template
echo "Building $template template"
done
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not doing anything other than looping over the folders we can add the commands to build the python templates in there so we check everything can be built

@@ -1,6 +1,6 @@
#!/bin/bash

maven_templates='HelloWorld'
maven_templates=$(ls -d */ | sed 's/\//\n/g')
Copy link
Member

Choose a reason for hiding this comment

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

Nit: This change assumes that all the templates inside Java can be built with maven. This will have to be modified if things change and that is also fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct the current one is assuming the same. This is just improving it so we don't have to add the names of the folders every time we add a new template

Copy link
Member

Choose a reason for hiding this comment

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

On the other hand, wondering is we should build these with gdk commands as that's we expect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmmm definitely gdk commands I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will hold off on trying to do this. Right now the gdk does not allow you to specify the path to the artifact and for the OTF framework it is inside one of the submodules

python_templates=$(ls -d */ | sed 's/\//\n/g')
current_directory=`pwd`
for template in $maven_templates; do
Copy link
Member

Choose a reason for hiding this comment

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

should be python_templates if we still wanna keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch

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.

3 participants