Skip to content

Global declaration in extracted method#392

Merged
lieryan merged 26 commits into
python-rope:masterfrom
climbus:global_declaration
Sep 18, 2021
Merged

Global declaration in extracted method#392
lieryan merged 26 commits into
python-rope:masterfrom
climbus:global_declaration

Conversation

@climbus

@climbus climbus commented Sep 17, 2021

Copy link
Copy Markdown
Contributor

316

Added support for global expression.

  • test task list checker

@climbus

climbus commented Sep 17, 2021

Copy link
Copy Markdown
Contributor Author

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

@lieryan

lieryan commented Sep 17, 2021

Copy link
Copy Markdown
Member

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

climbus commented Sep 17, 2021

Copy link
Copy Markdown
Contributor Author

Same here #391

@lieryan

lieryan commented Sep 17, 2021

Copy link
Copy Markdown
Member

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

climbus commented Sep 17, 2021

Copy link
Copy Markdown
Contributor Author

! [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

lieryan commented Sep 17, 2021

Copy link
Copy Markdown
Member

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

climbus commented Sep 17, 2021

Copy link
Copy Markdown
Contributor Author

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

lieryan commented Sep 17, 2021

Copy link
Copy Markdown
Member

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

Comment thread rope/refactor/extract.py Outdated

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

Copy link
Copy Markdown
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

climbus commented Sep 18, 2021

Copy link
Copy Markdown
Contributor Author

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

climbus commented Sep 18, 2021

Copy link
Copy Markdown
Contributor Author

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

lieryan commented Sep 18, 2021

Copy link
Copy Markdown
Member

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.

2 participants