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

Global declaration in extracted method #392

Merged
merged 26 commits into from Sep 18, 2021

Conversation

climbus
Copy link
Contributor

@climbus climbus commented Sep 17, 2021

316

Added support for global expression.

  • test task list checker

@climbus
Copy link
Contributor Author

climbus commented Sep 17, 2021

@lieryan Can you tell me why task is failing? Thanks

@lieryan
Copy link
Member

lieryan commented Sep 17, 2021

It shouldn't fail. That's a new Github Actions I added recently to enforce that all task list must be checked before merging. Your PR doesn't contain any task list, so it should not have failed; it seemed to be failing because of an unrelated issue because the task list don't have permission to access your fork or something like that.

I'll try and see if there's other different task list plugins that doesn't fail in such situations.

@climbus
Copy link
Contributor Author

climbus commented Sep 17, 2021

Same here #391

@lieryan
Copy link
Member

lieryan commented Sep 17, 2021

Hi @climbus, I made some changes to Github Actions that should hopefully resolve the task-completed-checker issue. Can you check if this issue is still there after you pull the latest master into this branch?

@climbus
Copy link
Contributor Author

climbus commented Sep 17, 2021

! [remote rejected] global_declaration -> global_declaration (refusing to allow a Personal Access Token to create or update workflow .github/workflows/task-completed-checker-action.yml without workflow scope)

@lieryan
Copy link
Member

lieryan commented Sep 17, 2021

Hi thanks for trying that out. Can you try rebasing this branch on top of the latest master and then using force push, something like this:

$ git fetch
$ git rebase origin/master global_declaration --onto origin/master
# ... resolve any rebase conflicts ...
$ git push --force

@climbus
Copy link
Contributor Author

climbus commented Sep 17, 2021

Unfortunately still errors
! [remote rejected] global_declaration -> global_declaration (refusing to allow a Personal Access Token to create or update workflow .github/workflows/task-completed-checker-action.yml without workflow scope)

! [remote rejected] master -> master (refusing to allow a Personal Access Token to create or update workflow .github/workflows/task-completed-checker-action.yml without workflow scope)

@lieryan
Copy link
Member

lieryan commented Sep 17, 2021

Can you try if making a new pull request from the rebased branch works?


def _Global(self, node):
for name in node.names:
self.globals_.add(name)
Copy link
Member

Choose a reason for hiding this comment

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

Would doing self.globals_.add(*node.names) like the other _Global work here as well?

@climbus
Copy link
Contributor Author

climbus commented Sep 18, 2021

Can you try if making a new pull request from the rebased branch works?

After rebase I can't push any commit in any branch on my fork. When I remove task check file, I can push again.

@climbus
Copy link
Contributor Author

climbus commented Sep 18, 2021

Can you try if making a new pull request from the rebased branch works?

After rebase I can't push any commit in any branch on my fork. When I remove task check file, I can push again.

Ok forget it. I had wrong permissions on token. Now it works fine. Thanks for help.

@lieryan lieryan added this to the 0.21.x milestone Sep 18, 2021
@lieryan lieryan merged commit edbacda into python-rope:master Sep 18, 2021
@lieryan
Copy link
Member

lieryan commented Sep 18, 2021

Hi @climbus, great work on this one as usual 💯

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.

None yet

2 participants