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

What's new and readme: we are moving to Python 3.7 #9942

Merged
merged 12 commits into from
Aug 5, 2019

Conversation

josephsl
Copy link
Collaborator

@josephsl josephsl commented Jul 17, 2019

Link to issue number:

Summary of the issue:

What's new and readme still mentions Python 2.7, and in case of what's new, changes were needed.

Description of how this pull request fixes the issue:

Edits include mentioning Python 3.7 in both readme and what's new, as well as fixing things here and there in what's new/Threshold section. See the below section regarding work included in what's new and readme through this pull request and other Threshold work.

Testing performed:

None

Known issues with pull request:

Some parts are missing, such as filling in which issue introduced support for pyserial 3.4 and other things. What's new and readme are subject to change based on comments.

Change log entry:

None

Pull requests addressed by this work

Items with a checkmark are mentioned in what's new. Merge messages are ignored.

Speech refactor

Python 3.7

No entry necessary

@josephsl josephsl requested a review from feerrenrut July 17, 2019 02:34
@josephsl
Copy link
Collaborator Author

Hi all,

Comments from everyone who has worked on or commented on Python 3 are appreciated, especially from Reef, @jcsteh, and @LeonarddeR.

Thanks,

@josephsl
Copy link
Collaborator Author

josephsl commented Jul 17, 2019 via email

Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

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

You can change the threshold release heading to 2019.3

@josephsl
Copy link
Collaborator Author

josephsl commented Jul 17, 2019 via email

@josephsl
Copy link
Collaborator Author

Hi,

Should we mention textUtils?

Thanks.

@LeonarddeR
Copy link
Collaborator

LeonarddeR commented Jul 17, 2019 via email

@josephsl
Copy link
Collaborator Author

Hi,

Regarding text utils, I have nothing to say - can't come up with a descriptive changelog for developers at the moment.

Anything else from our PR pool? If not, I'll switch the PR to review mode.

Thanks.

@josephsl josephsl marked this pull request as ready for review July 17, 2019 23:39
@LeonarddeR
Copy link
Collaborator

How about something within the lines of:

@josephsl
Copy link
Collaborator Author

Hi,

Text utils: done.

Thanks.

Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

This is looking pretty good. Please update the description to cover which PR's (to threshold / threshold staging) have been covered by this. You can get most of the way there by copying entries from git log on the threshold branch. I suggest using the checkbox markdown:

- [ ] some PR not covered
- [x] some PR covered

@josephsl
Copy link
Collaborator Author

Hi,

I'll use git shortlog to edit the description.

Thanks.

@josephsl
Copy link
Collaborator Author

Hi,

To @LeonarddeR: whenever you are ready, please take a look at this PR. Thanks.

@feerrenrut
Copy link
Contributor

Thanks @josephsl, I have updated the description so that they render as a checkboxes. It looks like there are still many PR's that need consideration.

@josephsl
Copy link
Collaborator Author

josephsl commented Jul 23, 2019 via email

@feerrenrut
Copy link
Contributor

If no one volunteers before tomorrow (25-July 0600 UTC+0) , then I'll finish this off.

Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

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

I found two sentences where has should be have IMO. Could be wrong for the latter, though.

- config.ConfigManager.getConfigValidationParameter is removed. A bug in getConfigValidation has been fixed in order for it being able to fully replace getConfigValidationParameter
- inputCore.InputGesture.logIdentifier property has been removed. Now use inputCore.InputGesture.identifiers[0] instead.
- textInfos.Point and textInfos.Rect has been removed and replaced by locationHelper.Point and locationHelper.RectLTRB, respectively.
- braille.BrailleHandler._get/set_tether has been removed. Use getTether and setTether respectively.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- braille.BrailleHandler._get/set_tether has been removed. Use getTether and setTether respectively.
- braille.BrailleHandler._get/set_tether have been removed. Use getTether and setTether respectively.

@josephsl
Copy link
Collaborator Author

josephsl commented Jul 24, 2019 via email

@josephsl
Copy link
Collaborator Author

Hi,

Let me at least commit the requested edits, and then I think we should decide our next steps.

Thanks.

@feerrenrut
Copy link
Contributor

I'd like to get this finished off and merged in, are you going to continue with it @josephsl? I don't think I will hold back threshold any longer.

@josephsl
Copy link
Collaborator Author

josephsl commented Jul 25, 2019 via email

@josephsl
Copy link
Collaborator Author

Hi,

Clarification: I'm done with this one.

Thanks.

feerrenrut added a commit that referenced this pull request Jul 25, 2019
Cherry picked changes from #9942

Closes #8421
@LeonarddeR
Copy link
Collaborator

@josephsl: could you please do a base branch switch for this?

@josephsl josephsl changed the base branch from threshold to changesFileLineEndingsChanged_master July 27, 2019 15:42
@josephsl josephsl changed the base branch from changesFileLineEndingsChanged_master to master July 27, 2019 15:43
josephsl added 3 commits July 27, 2019 08:44
What's new entry suggested by Leonard de Ruijter (Babbage).
Reviewed by Leonard de Ruijter (Babbage): has -> have for two entries.
@josephsl josephsl force-pushed the py3_docsAndReadme branch from 1cd5b05 to fc7e441 Compare July 27, 2019 15:45
@josephsl
Copy link
Collaborator Author

Done.

@derekriemer
Copy link
Collaborator

Should this be merged?

@josephsl
Copy link
Collaborator Author

josephsl commented Aug 4, 2019 via email

@feerrenrut
Copy link
Contributor

@derekriemer The checklist for PR's needs to be completed, I suspect many of these do not need an entry. This is on my list of things to do, but hasn't yet received priority.

@feerrenrut
Copy link
Contributor

I have gone through the unchecked items and added entries.

I also re-arranged the current items in the changes file.

@feerrenrut feerrenrut dismissed LeonarddeR’s stale review August 5, 2019 20:26

Changes were addressed

@feerrenrut feerrenrut merged commit ddf27fa into nvaccess:master Aug 5, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Aug 5, 2019
@josephsl josephsl deleted the py3_docsAndReadme branch September 16, 2019 01:57
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.

Update changes file on threshold branch
7 participants