search

iqm-finland / KQCircuits

KLayout Python library for integrated quantum circuit design.
GNU General Public License v3.0
144 stars 73 forks source link

Added Vim command for `create_element_from_path.py` #55

Closed AVDiv closed 1 year ago

AVDiv commented 1 year ago

48

Added Vim/NeoVim command for create_element_from_path.py with instructions to setup. Please notify if any changes are needed to be done.

cla-bot[bot] commented 1 year ago

Thank you for your pull request and welcome to our community.

We require contributors to sign our Contributor License Agreement (CLA), and it seems that the following contributors have not yet signed it: @AVDiv.

In order for us to review and merge your code, please sign the CLA at meetiqm.com.

Once you have signed the CLA, write a comment here and we'll verify. Please note that it may take some time for us to verify it.

AVDiv commented 1 year ago

Hey, I just signed the CLA.

qpavsmi commented 1 year ago

Thank you for your contribution! Will try out the instructions now and come back to you on approval or improvement ideas!

qpavsmi commented 1 year ago

@cla-bot check

cla-bot[bot] commented 1 year ago

The cla-bot has been summoned, and re-checked this pull request!

AVDiv commented 1 year ago

Any improvements to be done? @qpavsmi

qpavsmi commented 1 year ago

Any improvements to be done? @qpavsmi

Hi, sorry, I thought my review went public but apparently it didnt: https://github.com/iqm-finland/KQCircuits/pull/55#discussion_r1224041294

AVDiv commented 1 year ago

Any improvements to be done? @qpavsmi

Hi, sorry, I thought my review went public but apparently it didnt: #55 (comment)

No issues👍

AVDiv commented 1 year ago

Right, done with it. Anything else to improve? @qpavsmi

qpavsmi commented 1 year ago

Right, done with it. Anything else to improve? @qpavsmi

Thank you for the update, will check it when I get home

qpavsmi commented 1 year ago

Right, done with it. Anything else to improve? @qpavsmi

Thank you for the update, will check it when I get home

Looking at the diffs on my phone they look good, thank you! Please squash the three commits into one commit with a descriptive commit message (message for first commit was good enough). It will probably require force push.

AVDiv commented 1 year ago

Right, done with it. Anything else to improve? @qpavsmi

Thank you for the update, will check it when I get home

Looking at the diffs on my phone they look good, thank you! Please squash the three commits into one commit with a descriptive commit message (message for first commit was good enough). It will probably require force push.

Okay

AVDiv commented 1 year ago

I think starting off with a new PR would be nice, right?

qpavsmi commented 1 year ago

I think starting off with a new PR would be nice, right?

Actually dont worry about squashing, seems like when merging pull requests this is an option for me to do. So just leave this pr as it is and I will merge it a later time. I will do a last check on it when I get home and then assign you to the issue and close it so you get the bounty :) I will also make sure your git credentials will be present in our git history for contributing in this way

AVDiv commented 1 year ago

I think starting off with a new PR would be nice, right?

Actually dont worry about

OK

qpavsmi commented 1 year ago

I think starting off with a new PR would be nice, right?

Actually dont worry about

OK

I updated the last message :) anyway you could check some static analysis failures (just pylint stuff) here: https://github.com/iqm-finland/KQCircuits/actions/runs/5231064161/jobs/9444987678?pr=55 some lines were too long, so those can be fixed before merge

AVDiv commented 1 year ago

I think starting off with a new PR would be nice, right?

Actually dont worry about

OK

I updated the last message :) anyway you could check some static analysis failures (just pylint stuff) here: https://github.com/iqm-finland/KQCircuits/actions/runs/5231064161/jobs/9444987678?pr=55 some lines were too long, so those can be fixed before merge

yep, just noticed it & now I'm fixing it.

AVDiv commented 1 year ago

Right fixed those

AVDiv commented 1 year ago

Approving by comment, and will mark you as a receiver for the bounty. Depending on whether I'll have time tomorrow or early next week I will merge this as part of our official branch, there is a process for it and I need to be careful. Thank you very much for the contribution and good job!

Thank you, grateful to work with you.🙌

AVDiv commented 1 year ago

hi @qpavsmi , Can you also mark the issue as done on unitaryhack

qpavsmi commented 1 year ago

hi @qpavsmi , Can you also mark the issue as done on unitaryhack

Theyre supposed to have an automatic system for that, not sure why it hasnt noticed your bounty yet. Ive reached out the unitaryhack team about this.

qpavsmi commented 1 year ago

hi @qpavsmi , Can you also mark the issue as done on unitaryhack

Theyre supposed to have an automatic system for that, not sure why it hasnt noticed your bounty yet. Ive reached out the unitaryhack team about this.

Now its marked as done in UH site

AVDiv commented 1 year ago

Theyre supposed to have an automatic system for that, not sure why it hasnt noticed your bounty yet. Ive reached out the unitaryhack team about this.

Yep, I too thought it works that way looks like there is an issue, anyways thank you for marking this manually.😁

qpavsmi commented 1 year ago

Merged manually, closing PR