-
Notifications
You must be signed in to change notification settings - Fork 57
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
fix : #510, Removed package_to_check variable and updated references #511
fix : #510, Removed package_to_check variable and updated references #511
Conversation
…ences Signed-off-by: Shobhit Kumar <[email protected]>
…957/ecosystem into remove-package-to-check
Do not hesitate to ping me when it's ok for you :) |
Hey, what should I do, how can this can be merged?, I'm a beginner to this open-source community...please help me.. |
For merging we'll do it for you :) just continue to commit and you think your PR is ready just ping me I'll review it and then I'll ping someone who can merge it. Here you cleaned the tox tpl, you can as well delete wherever this var is called in the code. |
So, should I do that too?, like should I delete the var where ever it is in the code? |
Yes :) |
…ced it with qiskit Signed-off-by: Shobhit Kumar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you renamed everywhere the var to qiskit
?
You need to delete this var everywhere
Oops!, Very Sorry..my bad, I'll do that again.. |
Signed-off-by: Shobhit Kumar <[email protected]>
Signed-off-by: Shobhit Kumar <[email protected]>
Signed-off-by: Shobhit Kumar <[email protected]>
ecosystem/manager.py
Outdated
<<<<<<< HEAD | ||
======= | ||
qiskit: str, | ||
>>>>>>> 2b284cffea465cb413ac35b655efce9f5be03546 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, it looks like you have a lot of merge conflicts still like this, if you look through the "Files changed" tab on GitHub.
Thanks for your contribution by the way! We appreciate your help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, sorry for this..actually i'm a beginner here..learning and applying things. So I made this mistake, so sorry for that...will push the pr again in couple of minutes...Thanks for providing me this opportunity bro.
Signed-off-by: Shobhit Kumar <[email protected]>
Hey bro.. You can check, if I again made any mistake, please tell me bro.. I'll solve it and push the commit again... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! I'll wait for @mickahell to review.
please tell me bro
Gentle tip: it's better to not use "bro" when interacting with people in developer spaces like GitHub and open source. It's really casual, whereas we try to be a little more professional with GitHub.
Also not everyone identifies as male, and it's not safe to assume how someone identifies. For example, I'm non-binary so "bro" doesn't describe me.
Yep!, I'll follow the tip. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah seems good to me too :)
@Eric-Arellano could you merge it please 🚀
Fix: #510
Removed package_to_check variable and updated according to the reference.