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

chore: update package config #1448

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

vorel99
Copy link
Contributor

@vorel99 vorel99 commented Sep 17, 2023

In python world, new standard for package configuration is pyproject.toml. So I moved all static config from setup.py to pyproject.toml.

  • move all static package setup from setup.py to pyproject.toml
  • remove all requirements files
  • update gitlab actions
    • replace all requirements files usage

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2023

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.53%. Comparing base (3d4e482) to head (8632412).
Report is 76 commits behind head on develop.

Current head 8632412 differs from pull request most recent head d53bbe7

Please upload reports for the commit d53bbe7 to get more accurate results.

Files Patch % Lines
setup.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1448      +/-   ##
===========================================
- Coverage    90.01%   89.53%   -0.48%     
===========================================
  Files          194      194              
  Lines         6368     6316      -52     
===========================================
- Hits          5732     5655      -77     
- Misses         636      661      +25     
Flag Coverage Δ
py3.8-ubuntu-22.04-pandas 89.53% <75.00%> (-0.48%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fabclmnt
Copy link
Contributor

Hi @vorel99 ,

once again than you for your contribution :)

I've already requested the review from the team!

@vorel99 vorel99 changed the title refactor: update package config feat: update package config Sep 19, 2023
.gitignore Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
requirements-dev.txt Outdated Show resolved Hide resolved
requirements-spark.txt Outdated Show resolved Hide resolved
requirements-test.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
@vascoalramos
Copy link
Contributor

vascoalramos commented Sep 20, 2023

Hi @vorel99.

Thank you for your work in the migration to pyproject.toml.
I left some change requests mostly related to the makefile and dependency options.
The primary reason for most of the requests relates to keeping consistency between all of our open-source projects.

@vorel99 vorel99 marked this pull request as ready for review September 20, 2023 17:30
Copy link
Contributor

@vascoalramos vascoalramos left a comment

Choose a reason for hiding this comment

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

Everything else seems fine to me. @portellaa, what do you think?

Makefile Outdated Show resolved Hide resolved
@vorel99
Copy link
Contributor Author

vorel99 commented Oct 9, 2023

hello,
@vascoalramos @portellaa what is the status of MR? Is there any blocker to be resolved?

@aquemy aquemy force-pushed the develop branch 2 times, most recently from 6a3342a to 8f4f622 Compare October 10, 2023 10:25
@portellaa
Copy link
Member

Hi @vorel99

Thanks for your PR.

Unfortunately we don't have the time right now to move forward with this, we kindly ask you to give us some more time.
Given the amount of the changes, it breaks some internal alignments that we will need to work with you, but right now we can't focus on this.

Sorry for this.
We will get in touch as soon as we can pick this.

Cheers

@vorel99
Copy link
Contributor Author

vorel99 commented Oct 11, 2023

Thanks for reply @portellaa,
there wasn't that many of changes (its probably result of force-push from aquemy).
Changes were just in 15 files with 189 additions and 171 deletions.
There is an example of pull request, what it looked like when it was created and vascoalramos did review.
vorel99#78

I can rebase my branch to new develop, when you are ready.

Regards,
JC

@vorel99
Copy link
Contributor Author

vorel99 commented Jun 7, 2024

Hello @fabclmnt,
would it be possible to reassign reviewers? The pull request was inactive for almost a year due to waiting for a message from one of current reviewers.

Thank you,
JC

@fabclmnt
Copy link
Contributor

Hi @vorel99,

just reviewed the PR. The changes seem to be aligned!

There are a few conflicting issues that need to be sorted so we can proceed with merge.

@fabclmnt fabclmnt requested review from vascoalramos and removed request for portellaa and vascoalramos September 17, 2024 15:15
@fabclmnt fabclmnt changed the title feat: update package config chore: update package config Sep 17, 2024
@vorel99
Copy link
Contributor Author

vorel99 commented Sep 23, 2024

Hi @fabclmnt ,
I rebased feature branch to develop.
PR is ready for merge

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.

5 participants