Skip to content

Remove required_ from core & added tests to ensure both are working #169

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

agam1092005
Copy link

Solved issue #166
Kindly check if issue is solved and merge the PR

Agampreet Singh

Copy link
Contributor

@samriddhi99 samriddhi99 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Everything looks great, but can you fix the linting of the code?

Copy link

@AkashS0510
Copy link
Collaborator

@agam1092005 Can you fix the linting so that we can merge this PR? Thanks!!

@agam1092005
Copy link
Author

@agam1092005 Can you fix the linting so that we can merge this PR? Thanks!!

Sorry, but unable to understand & find the issue

@samriddhi99
Copy link
Contributor

@agam1092005 Can you fix the linting so that we can merge this PR? Thanks!!

Sorry, but unable to understand & find the issue

Hey there!

No worries if "linting" sounded confusing! Basically, we just need to clean up the code's formatting so it's neat and follows standard style guidelines.

Tirith uses a tool called black. Here’s how you can do it:

  1. Install Black (if you don’t have it already) by running this command:

    pip install black
    
  2. Once installed, just run this in your terminal:

    black <your_filename.py>
    

    Replace <your_filename.py> with the actual file name you're working on.

That’s it! Black will automatically fix the formatting for you, so you don’t have to worry about the details. Let me know if you need help with anything else! 😊

@AkashS0510
Copy link
Collaborator

@agam1092005 Have you been able to fix it?

Copy link

sonarqubecloud bot commented Oct 4, 2024

Copy link
Member

@refeed refeed left a comment

Choose a reason for hiding this comment

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

This looks good, but only one thing left: backward compatibility. Could you modify the code in a way that it will still accept the old required_provider when the user provided that? Thanks

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.

4 participants