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

replace time.clock with perf_counter for py3 #638

Closed
wants to merge 1 commit into from

Conversation

@frostming
Copy link

@frostming frostming commented Sep 14, 2018

Replace the timer to suppress the deprecation warning.

flask_sqlalchemy/__init__.py Outdated Show resolved Hide resolved
Loading
@mfreeborn

This comment was marked as off-topic.

@rsyring
Copy link
Contributor

@rsyring rsyring commented Mar 9, 2019

#662 uses if sys.version_info >= (3, 3): while this PR uses sys.version_info[:2] >= (3, 3):. I believe both would work given tuple comparison semantics and the former is just a bit cleaner.

Also, we don't currently have CI setup and I don't have easy access to a Windows machine. Will someone manually confirm that this code works as expected on Windows for Python 2.7 and a version of Python3 > 3.3.

Loading

@rsyring rsyring added this to the 2.x milestone Mar 9, 2019
@frostming
Copy link
Author

@frostming frostming commented Mar 11, 2019

@rsyring I squashed the commits and tested on Windows PC, with Python 3.7.2 and Python 2.7.15, all pass. Thanks

Loading

@rsyring
Copy link
Contributor

@rsyring rsyring commented Mar 11, 2019

Great, thank you. I'll update the milestone to 2.4 as I see no reason to delay this.

Loading

@rsyring rsyring removed this from the 2.x milestone Mar 11, 2019
@rsyring rsyring added this to the 2.4 milestone Mar 11, 2019
@rsyring
Copy link
Contributor

@rsyring rsyring commented Mar 14, 2019

Would you mind rebasing this on the current 2.x-maintenance branch and change the PR to merge into that branch? I believe that is the last step needed before I can get this merged.

Thanks.

Loading

@davidism
Copy link
Member

@davidism davidism commented Mar 14, 2019

The command for that would be git rebase --onto origin/2.x-maintenance origin/master deprecate-timer, then git push -f, then edit the PR here to change the target.

@rsyring I usually do this myself, as it's not a common thing for most PR authors to know.

Loading

@rsyring
Copy link
Contributor

@rsyring rsyring commented Mar 14, 2019

@davidism ok, I'll take care of it then. Thanks @frostming for the submission.

Loading

rsyring added a commit that referenced this issue Mar 14, 2019
@rsyring

This comment has been hidden.

@rsyring rsyring closed this Mar 14, 2019
@davidism davidism reopened this Mar 14, 2019
@davidism davidism force-pushed the deprecate-timer branch from 2396a0a to 51637e2 Mar 14, 2019
@davidism davidism changed the base branch from master to 2.x-maintenance Mar 14, 2019
rsyring added a commit that referenced this issue Mar 14, 2019
@rsyring
Copy link
Contributor

@rsyring rsyring commented Mar 14, 2019

@davidism I merged this PR to the maintenance branch, after rebasing again, but the PR didn't pick up that the commit was merged. Let me know here or on Discord if you can tell what I did wrong.

Closing this PR, but it was merged.

Loading

@rsyring rsyring closed this Mar 14, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants