Skip to content

Conversation

@delterr
Copy link

@delterr delterr commented Sep 12, 2024

Addresses #69

@delterr
Copy link
Author

delterr commented Sep 12, 2024

not ready to merge yet as i had set the command to specifically run to ons_python but need to generalise to all module names with jinja

Comment on lines +41 to +43
.PHONY: run
run: ## Run the application
poetry run python {{module_name}}
Copy link
Collaborator

@MebinAbraham MebinAbraham Oct 25, 2024

Choose a reason for hiding this comment

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

Thanks for the PR.

  1. This is the wrong file; you need to add this to https://github.com/ONSdigital/ons-python-template/blob/main/project_template/Makefile.jinja (templates makefile, not this repo's root template)

  2. You also need to make it agnostic to the package manager, so it should be:

{{ package_manager }} run python {{ module_name }}
  1. Please add this new __main__.py file to https://github.com/ONSdigital/ons-python-template#structure

Comment on lines +1 to +8
from calculator import Calculator
calc = Calculator()

calc.add(5)

calc.subtract(2)

print(calc.cumulative_total)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would currently raise lint failures.

Suggested change
from calculator import Calculator
calc = Calculator()
calc.add(5)
calc.subtract(2)
print(calc.cumulative_total)
from calculator import Calculator
calc = Calculator()
calc.add(5)
calc.subtract(2)
print(calc.cumulative_total)

@sanjeevz3009
Copy link
Collaborator

@delterr If you address the above comments, we can progress this

@sanjeevz3009
Copy link
Collaborator

Closing the PR due to inacivity and the following PR superseeding the current one: #114.

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