fix: Fix issues with docs, makim/make, release, and CI #182

xmnlab · 2023-08-29T20:18:29Z

Pull Request description

This is a big PR that aims to fix a bunch of issues:

Implement properly the makim targets, including tests and documentation tasks
Fix main CI workflow and the Release workflow
Fix the semantic-release configuration
Fix the some URLs in the GitHub ISSUES templates that was pointing to the repository
Fix some configurations in the pyprojet.toml
Add tests for the automation tools to CI
Update cookiecutter.json and the base profile respect to makim and make
Update some python pinning from 3.8 to 3.8.1

How to test these changes

...

Pull Request checklists

This PR is a:

bug-fix
new feature
maintenance

About this PR:

it includes tests.
the tests are executed on CI.
the tests generate log file(s) (path).
pre-commit hooks were executed locally.
this PR requires a project documentation update.

Author's checklist:

I have reviewed the changes and it contains no misspelling.
The code is well commented, especially in the parts that contain more complexity.
New and old tests passed locally.

Additional information

Reviewer's checklist

Copy and paste this template for your review's note:

## Reviewer's Checklist

- [ ] I managed to reproduce the problem locally from the `main` branch
- [ ] I managed to test the new changes locally
- [ ] I confirm that the issues mentioned were fixed/resolved .

Saransh-cpp

Thanks for the great work, @xmnlab! Some minor comments below. Also, should a user be allowed to user be allowed to use both makim and Make together? If not, then we should add a condition here -

scicookie/src/scicookie/hooks/post_gen_project.py

Lines 248 to 253 in 0c5337e

    
           def validation(): 
        
               if USE_BLUE and USE_BLACK: 
        
                   raise Exception( 
        
                       "The libs Blue and Black were selected, but you need to choose " 
        
                       "just one of them." 
        
                   )

Saransh-cpp · 2023-08-30T01:32:23Z

src/scicookie/{{cookiecutter.project_slug}}/.makim.yaml

+      linter:
+        help: Run linter tools
+        run: |
+          pre-commit install


This isn't actually required, given that this file invoked pre-commit explicitly using pre-commit run --all-files --verbose. Having this is not a big issue too, does not take a lot of time to add pre-commit hooks as git hooks 🙂

yep, the idea is to force the user to install the pre-commit hook locally ... maybe it is not the best approach, I am currently investigating some better approach for doing that.

for poetry I found this: https://pypi.org/project/poetry-pre-commit-plugin/
but it seems it is not mature yet (all its tests are not executed: https://github.com/vstrimaitis/poetry-pre-commit-plugin/blob/master/tests/test_plugin.py

maybe we could do it as a post-gen step .. so it could work for all the projects .. but we would need to create the environment first ... so not sure if it is a good idea or not

nodejs projects are very nice in this topic .. because when you install it for development, it automatically installs pre-commit hook.

so if it is not a problem, I would keep it for now, and in the meantime I will try to figure out a good approach that would works for all the build-systems

src/scicookie/{{cookiecutter.project_slug}}/Makefile

Saransh-cpp · 2023-08-30T01:35:49Z

src/scicookie/{{cookiecutter.project_slug}}/Makefile


 .PHONY:lint
 lint:
+	pre-commit install


This can be remove, but again, having it does not hurt a lot

as I mentioned before, I would keep this for now, until I come with a better solution to "force" the installation of pre-commit hooks from the beginning

Saransh-cpp · 2023-08-30T01:38:33Z

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml

@@ -35,7 +35,7 @@ classifiers = [
  "Typing :: Typed",


Not related to this PR, but just noticed that these lines should be in a if cookiecutter.use_mypy == "yes" condition (in all the build backend pyproject.toml files).

oh I see, I will open an issue for that. thanks!

Anavelyz

Great, A thousand thanks! 🚀

Leave some comments for the guide.

docs/guide.md

Anavelyz · 2023-08-30T15:47:52Z

docs/guide.md

@@ -770,18 +770,18 @@ by selecting the option `None` (this is the default option).

 ## Automation Tools


Include it in the structure, please

sorry, I am not following you here, could you give more details please?

After that line:

scicookie/docs/guide.md

Line 26 in 0c5337e

- [Integration with DevOps tools](#integration-with-devops-tools)

should be:

- [Automation tools](#automation-tools)

oh got it, thank you for the explanation, I am going to add it now. thanks!

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Co-authored-by: Anavelyz Perez <42752529+Anavelyz@users.noreply.github.com>

xmnlab · 2023-08-30T19:47:00Z

Thanks for the great work, @xmnlab! Some minor comments below. Also, should a user be allowed to user be allowed to use both makim and Make together? If not, then we should add a condition here -

scicookie/src/scicookie/hooks/post_gen_project.py

Lines 248 to 253 in 0c5337e

def validation():

if USE_BLUE and USE_BLACK:

raise Exception(

"The libs Blue and Black were selected, but you need to choose "

"just one of them."

)

for now it is used for the same thing, but the user maybe would like to use make for a different purpose, so at the end of the project creation the user would need to rewrite the Makefile

scicookie uses both for the same purpose, so that is why makim excludes some usage of make if both are selected, but (in the case both are selected) we should ensure that the make is installed and the Makefile is created, but makim should be used in workflows (because make will be probably used for another thing)

does it make sense?

xmnlab · 2023-08-30T19:52:23Z

It seems that currently there is no code for removing the makim file or makefile regarding to the user selection.
I will fix that in this PR too. I am on it now

xmnlab · 2023-08-30T20:08:57Z

@Saransh-cpp @Anavelyz I addressed the points you made and added a new change that removes Makefile or .makim.yaml if their tools are not selected.

xmnlab · 2023-08-30T21:29:06Z

I also fixed an issue with hatch as well in the smoke tests.
@Saransh-cpp @Anavelyz let me know if you have any other comments here or if it is ready to go :)

Saransh-cpp

Awesome, looks good now! Thanks, @xmnlab!

xmnlab · 2023-08-31T01:57:00Z

@Saransh-cpp thanks for the review!

github-actions · 2023-08-31T02:09:20Z

🎉 This PR is included in version 0.6.2 🎉

The release is available on:

0.6.2
GitHub release

Your semantic-release bot 📦🚀

xmnlab added 7 commits August 29, 2023 16:18

fix: Fix issues with docs, makim/make, release, and CI

cc19dac

add automation to ci

6006f65

fix base and poetry pyproject.toml

873ac7b

fix issues with makim and mkdocs

b0a0884

fix style code issues in the test file

41ffee9

fix python pinning

40e7127

fix style code issues in the test file

886e1cd

xmnlab requested review from Anavelyz, ayeankit, Saransh-cpp and YurelyCamacho August 29, 2023 22:13

Saransh-cpp reviewed Aug 30, 2023

View reviewed changes

Anavelyz reviewed Aug 30, 2023

View reviewed changes

xmnlab and others added 3 commits August 30, 2023 15:20

Update src/scicookie/{{cookiecutter.project_slug}}/Makefile

3f99399

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update docs/guide.md

206d0d6

Update docs/guide.md

2a10c9f

Co-authored-by: Anavelyz Perez <42752529+Anavelyz@users.noreply.github.com>

Remove makefile or .makim.yaml if make or makim are not selected

09d4030

xmnlab added 2 commits August 30, 2023 16:12

add suggestion from @Anavelyz about the guide file.

8668e55

fix issues with hatch

0a5afd1

Saransh-cpp approved these changes Aug 31, 2023

View reviewed changes

xmnlab merged commit 12efe9a into osl-incubator:main Aug 31, 2023
11 checks passed

xmnlab deleted the fix-configurations branch August 31, 2023 02:05

github-actions bot added the released label Aug 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fix issues with docs, makim/make, release, and CI #182

fix: Fix issues with docs, makim/make, release, and CI #182

xmnlab commented Aug 29, 2023 •

edited

Loading

Saransh-cpp left a comment

Saransh-cpp Aug 30, 2023

xmnlab Aug 30, 2023

Saransh-cpp Aug 30, 2023

xmnlab Aug 30, 2023

Saransh-cpp Aug 30, 2023

xmnlab Aug 30, 2023

Anavelyz left a comment

Anavelyz Aug 30, 2023

xmnlab Aug 30, 2023

Anavelyz Aug 30, 2023

xmnlab Aug 30, 2023

xmnlab Aug 30, 2023

xmnlab commented Aug 30, 2023

xmnlab commented Aug 30, 2023

xmnlab commented Aug 30, 2023

xmnlab commented Aug 30, 2023

Saransh-cpp left a comment

xmnlab commented Aug 31, 2023

github-actions bot commented Aug 31, 2023

	def validation():
	if USE_BLUE and USE_BLACK:
	raise Exception(
	"The libs Blue and Black were selected, but you need to choose "
	"just one of them."
	)

		@@ -770,18 +770,18 @@ by selecting the option `None` (this is the default option).

		## Automation Tools

fix: Fix issues with docs, makim/make, release, and CI #182

fix: Fix issues with docs, makim/make, release, and CI #182

Conversation

xmnlab commented Aug 29, 2023 • edited Loading

Pull Request description

How to test these changes

Pull Request checklists

Additional information

Reviewer's checklist

Saransh-cpp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anavelyz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xmnlab commented Aug 30, 2023

xmnlab commented Aug 30, 2023

xmnlab commented Aug 30, 2023

xmnlab commented Aug 30, 2023

Saransh-cpp left a comment

Choose a reason for hiding this comment

xmnlab commented Aug 31, 2023

github-actions bot commented Aug 31, 2023

xmnlab commented Aug 29, 2023 •

edited

Loading