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(project information): adding missing information #149

Merged
merged 11 commits into from
Jul 20, 2023

Conversation

YurelyCamacho
Copy link
Collaborator

@YurelyCamacho YurelyCamacho commented Jul 19, 2023

Pull Request description

Adding missing information after the project creation and fix bugs 3/5

  • Description using the one given via TUI in pyproject.toml
  • Adding packages entry in pyproject.toml
  • Fix a block with wrong indentation in the release workflow in the template

Solve #147

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 .

@YurelyCamacho
Copy link
Collaborator Author

YurelyCamacho commented Jul 20, 2023

@xmnlab In the case of setuptools, Do you consider it necessary to add this?: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html

Thanks @Anavelyz for your comment

@YurelyCamacho YurelyCamacho marked this pull request as ready for review July 20, 2023 17:24
@YurelyCamacho YurelyCamacho requested a review from xmnlab July 20, 2023 17:24
@xmnlab
Copy link
Member

xmnlab commented Jul 20, 2023

@xmnlab In the case of setuptools, Do you consider it necessary to add this?: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html

Thanks @Anavelyz for your comment

@YurelyCamacho could you give me more details about this?

@xmnlab
Copy link
Member

xmnlab commented Jul 20, 2023

I mean .. about your question

@xmnlab
Copy link
Member

xmnlab commented Jul 20, 2023

@YurelyCamacho in general it looks great! thanks for working on that.
just a question about the following points:

  • description is using the default one, instead of the one given via TUI
  • authors has the correct email, but the name is the default one, instead of the one given via TUI
  • The git user is not set correct (the name used is from the default, instead of the one given via TUI)

so basically, what that means is that at the end scicookie is not using the values specified in the inputs .. it uses just the default values .. for example if I type for the name "Luffy" at the end the ouput project will have "Rorona Zoro", instead of Luffy.

if you generate a new project, check the pyproject file and the git history inside the project with git log

could you check if that was resolved or if you can still see that issue please?
other than that, everything looks great :) thank you so much

@xmnlab
Copy link
Member

xmnlab commented Jul 20, 2023

also, I am totally fine to merge this PR as it is, and you can work on a follow-up to address the other points if anything else is missing.

@xmnlab
Copy link
Member

xmnlab commented Jul 20, 2023

oh just saw that you already mentioned about that in your initial comments.
ok, let's merge the PR. thank you so much

@xmnlab xmnlab merged commit 76ee498 into osl-incubator:main Jul 20, 2023
@YurelyCamacho YurelyCamacho deleted the documentation branch July 20, 2023 20:16
@github-actions
Copy link

🎉 This PR is included in version 0.4.0 🎉

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