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

perf(template): Improving the git workflow for the template #162

Merged
merged 4 commits into from
Oct 30, 2023

Conversation

Anavelyz
Copy link
Collaborator

@Anavelyz Anavelyz commented Aug 2, 2023

Pull Request description

  • Editing prepare_git function in src/scicookie/hooks/post_gen_project.py.
  • Editing files in src/scicookie/{{cookiecutter.project_slug}}/.github/workflows/
  • Fix an unnecessary space.
  • Adding virtualenv workflow
  • Updating guide.md

Solve #38

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.

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.

@Anavelyz Anavelyz requested a review from xmnlab August 7, 2023 03:08
Copy link
Member

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

@Anavelyz could you update/rebase your branch please?

@Anavelyz Anavelyz marked this pull request as ready for review August 8, 2023 20:46
@Anavelyz Anavelyz requested a review from xmnlab August 8, 2023 20:57
@xmnlab
Copy link
Member

xmnlab commented Aug 11, 2023

@Anavelyz thanks for working on that!!
could you rebase your branch please?

about the conda/virtualenv .. that is awesome ... we really to add this support! maybe we could have a quick meeting to discuss a bit more about that, because some backend already implement virtual env under the hood, for example hatch and poetry. I think that if we could keep the ci workflow in just one file, instead of splitting it into different folders it would be better

about the git stuff .. I think your approach is not much more complex (many lines) compared to the current approach ...
please check if there is anything extra that your PR handle that was not handled by my previous PR and keep it, otherwise I think it would be better to just rollback your changes about the git workflow.
again, really sorry about my last PR that implemented the git stuff ... I messed that up XD it was not my intention ... I just was in a mood to fix a bunch of things ... and I forgot about your PR .. really sorry

@Anavelyz
Copy link
Collaborator Author

😃
@xmnlab thank for your review.

could you rebase your branch please?

Yes, then I do. I should check your changes carefully, in my approach what I try to do is to check whether or not the URLs entered by the users exist.

about the conda/virtualenv .. that is awesome ... we really to add this support! maybe we could have a quick meeting to discuss a bit more about that, because some backend already implement virtual env under the hood, for example hatch and poetry. I think that if we could keep the ci workflow in just one file, instead of splitting it into different folders it would be better

👍🏾
Yes, I agree that a meeting is needed.

again, really sorry about my last PR that implemented the git stuff ... I messed that up XD it was not my intention ... I just was in a mood to fix a bunch of things ... and I forgot about your PR .. really sorry

Don't worry, It's great to see that there is meaningful input 🥳

@Anavelyz Anavelyz reopened this Oct 2, 2023
@Anavelyz Anavelyz marked this pull request as ready for review October 2, 2023 01:34
@Anavelyz
Copy link
Collaborator Author

Anavelyz commented Oct 2, 2023

@xmnlab Can you review it? Thank you!

@xmnlab
Copy link
Member

xmnlab commented Oct 30, 2023

@Anavelyz , sorry for taking that much time for reviewing this PR, I was on vacation.
it looks great, thanks for working on that!

@xmnlab xmnlab merged commit 7ab065e into osl-incubator:main Oct 30, 2023
11 checks passed
Copy link

github-actions bot commented Dec 8, 2023

🎉 This PR is included in version 0.6.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants