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

feat: project_id as property #129

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Conversation

OmAximani0
Copy link
Contributor

@OmAximani0 OmAximani0 commented Oct 9, 2023

For now it is just configured, I will push the implementation all over the code-base on this branch only.


ToDo's

For me(@OmAximani0) only as checklist. @andrii-bodnar You can add things that should be implemented with this PR

  • project_id as optional param for all API resources
  • Add test for client and other API resources
  • Update README.md

@OmAximani0 OmAximani0 marked this pull request as draft October 9, 2023 19:28
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2023

Codecov Report

Merging #129 (81302b7) into main (256c60a) will decrease coverage by 0.02%.
The diff coverage is 99.07%.

@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
- Coverage   99.20%   99.18%   -0.02%     
==========================================
  Files         146      146              
  Lines        5088     5425     +337     
  Branches      717      745      +28     
==========================================
+ Hits         5047     5380     +333     
- Misses         29       32       +3     
- Partials       12       13       +1     
Files Coverage Δ
...api/api_resources/abstract/tests/test_resources.py 100.00% <100.00%> (ø)
crowdin_api/api_resources/bundles/resource.py 100.00% <100.00%> (ø)
..._resources/bundles/tests/test_bundles_resources.py 100.00% <100.00%> (ø)
crowdin_api/api_resources/dictionaries/resource.py 100.00% <100.00%> (ø)
.../dictionaries/tests/test_dictionaries_resources.py 100.00% <100.00%> (ø)
...rowdin_api/api_resources/distributions/resource.py 100.00% <100.00%> (ø)
...istributions/tests/test_distributions_resources.py 100.00% <100.00%> (ø)
crowdin_api/api_resources/glossaries/resource.py 100.00% <100.00%> (ø)
...rces/glossaries/tests/test_glossaries_resources.py 100.00% <100.00%> (ø)
crowdin_api/api_resources/labels/resource.py 100.00% <100.00%> (ø)
... and 36 more

@andrii-bodnar andrii-bodnar added the hacktoberfest This issue welcomes contributions for Hacktoberfest label Oct 10, 2023
@OmAximani0
Copy link
Contributor Author

@andrii-bodnar Please checkout these changes and I have not done changes in tests but the builds are passing please suggest changes according

@OmAximani0
Copy link
Contributor Author

@andrii-bodnar Please checkout below resources in which I have made the changes according to previous changes suggested and please inform me that can I change all the other API resources according to these below resources?

@andrii-bodnar
Copy link
Member

@OmAximani0 this looks good to me. The only thing I'd suggest changing is:

projectId = projectId if projectId else self.get_project_id()

can be simplified:

projectId = projectId or self.get_project_id()

@OmAximani0
Copy link
Contributor Author

@OmAximani0 this looks good to me. The only thing I'd suggest changing is:

projectId = projectId if projectId else self.get_project_id()

can be simplified:

projectId = projectId or self.get_project_id()

@andrii-bodnar Fine then, I'll implement the line as you suggested and I'll also change the other API resources according to it 👍

@OmAximani0 OmAximani0 marked this pull request as ready for review October 22, 2023 18:20
@OmAximani0 OmAximani0 marked this pull request as draft October 24, 2023 08:53
@OmAximani0 OmAximani0 marked this pull request as ready for review October 24, 2023 12:17
Copy link
Member

@andrii-bodnar andrii-bodnar left a comment

Choose a reason for hiding this comment

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

@OmAximani0 thank you!

@andrii-bodnar andrii-bodnar merged commit 05ff99b into crowdin:main Oct 26, 2023
6 checks passed
@OmAximani0 OmAximani0 deleted the feat/property branch October 26, 2023 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest This issue welcomes contributions for Hacktoberfest hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add project_id property to client to set it once and skip this parameter in functions
3 participants