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

Git dependencies respect poetry.lock on install #2327

Closed
wants to merge 13 commits into from

Conversation

wakemaster39
Copy link

Pull Request Check List

Resolves: #2325

  • Added tests for changed code.
  • Updated documentation for changed code.

There is no documentation change required (I think)

@wakemaster39
Copy link
Author

There was a missing variable being passed in preventing me from getting master running locally in the new _execute_setup() function. Fix is included here.

Comment on lines 818 to 819
@classmethod
def _execute_setup(self):

Choose a reason for hiding this comment

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

It doesn't seem you are using the class itself. In this case it is better to use staticmethod:

Suggested change
@classmethod
def _execute_setup(self):
@staticmethod
def _execute_setup():

Copy link
Author

@wakemaster39 wakemaster39 May 1, 2020

Choose a reason for hiding this comment

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

this one wasn't me, I am not sure what to do here,

cls._execute_setup()

Master is broken right now and this was my best way of fixing it without touching anything else.

lpkg = self.__find_git_locked_version(pkg)

if lpkg:
print("lpkg", lpkg.source_reference)

Choose a reason for hiding this comment

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

Are these debug prints?

Copy link
Author

Choose a reason for hiding this comment

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

yep forgot to kil lthese

@al-muammar
Copy link

@sdispater, could we have your attention in this PR?

@2m
Copy link

2m commented Jul 14, 2020

@wakemaster39 could you rebase this PR?

@abn
Copy link
Member

abn commented Jul 15, 2020

@jihadik can you rework this on top of develop please?

@al-muammar
Copy link

Sorry, @abn, not sure what do you mean.

@kasteph
Copy link
Member

kasteph commented Jul 16, 2020

@jihadik branch off of develop instead of master 🙂

@wakemaster39 wakemaster39 changed the base branch from master to develop July 26, 2020 21:43
@wakemaster39
Copy link
Author

@2m @abn PR has been rebased and based off of development now.

@abn
Copy link
Member

abn commented Jul 26, 2020

@wakemaster39 there was a broader change that would fix this issue at #2327 this might get closed in favour of that one.

Also, apologies for not responding to this earlier. Since the last comments, we have switched to using master since the 1.0.10 release was done. The develop branch was merged into master. New features/fixes now go to master.

@wakemaster39
Copy link
Author

@abn thanks for getting back to me, I have adjusted this back to master. I think you meant to link a different issue though, as this is #2327 and I would be interested in seeing the broader change that would resolve this.

@abn
Copy link
Member

abn commented Jul 27, 2020

@wakemaster39 yes; I meant #2720 - copy-pasta! :)

@wakemaster39
Copy link
Author

wakemaster39 commented Jul 30, 2020

didn't push yet, but this is working again once #2722 is merged

@abn
Copy link
Member

abn commented Apr 5, 2021

Closing in favour of #3875.

@abn abn closed this Apr 5, 2021
Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git Dependencies do not respect lock file
5 participants