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

fix : #510, Removed package_to_check variable and updated references #511

Merged
merged 8 commits into from
Aug 31, 2023

Conversation

shobhit9957
Copy link
Contributor

Fix: #510
Removed package_to_check variable and updated according to the reference.

@CLAassistant
Copy link

CLAassistant commented Aug 25, 2023

CLA assistant check
All committers have signed the CLA.

@mickahell
Copy link
Collaborator

Do not hesitate to ping me when it's ok for you :)

@shobhit9957
Copy link
Contributor Author

Hey, what should I do, how can this can be merged?, I'm a beginner to this open-source community...please help me..

@mickahell
Copy link
Collaborator

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.

@shobhit9957
Copy link
Contributor Author

So, should I do that too?, like should I delete the var where ever it is in the code?

@mickahell
Copy link
Collaborator

So, should I do that too?, like should I delete the var where ever it is in the code?

Yes :)

Copy link
Collaborator

@mickahell mickahell left a 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

@shobhit9957
Copy link
Contributor Author

Oops!, Very Sorry..my bad, I'll do that again..

Comment on lines 327 to 330
<<<<<<< HEAD
=======
qiskit: str,
>>>>>>> 2b284cffea465cb413ac35b655efce9f5be03546
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

@shobhit9957
Copy link
Contributor Author

Hey bro.. You can check, if I again made any mistake, please tell me bro.. I'll solve it and push the commit again...

Copy link
Collaborator

@Eric-Arellano Eric-Arellano 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 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.

@shobhit9957
Copy link
Contributor Author

Yep!, I'll follow the tip.

Copy link
Collaborator

@mickahell mickahell left a 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 🚀

@Eric-Arellano Eric-Arellano merged commit f7182f9 into Qiskit:main Aug 31, 2023
4 checks passed
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.

Not need to use qiskit-terrain dev tests anymore
4 participants