Skip to content

Updates for change log2021.1 - #12463

Merged
feerrenrut merged 4 commits into
nvaccess:betafrom
CyrilleB79:changeLog2021.1
Jun 3, 2021
Merged

Updates for change log2021.1#12463
feerrenrut merged 4 commits into
nvaccess:betafrom
CyrilleB79:changeLog2021.1

Conversation

@CyrilleB79

Copy link
Copy Markdown
Contributor

Link to issue number:

None

Summary of the issue:

This PR aims to gather issues found in the beta stage.
The following issue have been found:

  1. There is a line in the change log for CLDR 38.1 update, but not for CLDR 39.0 update
  2. The blurb indicates that add-ons will need to have their manifest updated. This seems very devleopper oriented, not suitable for a blurb.

Description of how this pull request fixes the issue:

Modified the change log as follows:

  1. Indicate update to CLDR 39.0 rather than 38.1; but referenced both PRs (the one for 38.1 update and the one for 39.0 update)
  2. I have rewritten the blurb regarding add-ons to make it more end-user oriented. (Feel free to correct me, English is not may native language.) I have also put all the modifications in the first paragraph and put the note regarding the add-ons in a separate paragraph below.

Testing strategy:

Check the change log generated by appVeyor.

Known issues with pull request:

None

Change log entries:

None of course!

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual testing.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers

@CyrilleB79

Copy link
Copy Markdown
Contributor Author

Passing this PR as ready.
Please double check the following sentence:

Add-ons will need to be re-tested and authors will need to provide a new version certifying its compatibility with this release.

This seems grammatically incorrect (singular vs. plural...) to me but I would prefer to have the view of a native English speaker before modifying it. Thanks.

@CyrilleB79
CyrilleB79 marked this pull request as ready for review May 25, 2021 19:55
@CyrilleB79
CyrilleB79 requested a review from a team as a code owner May 25, 2021 19:55
@CyrilleB79
CyrilleB79 requested review from seanbudd and removed request for a team May 25, 2021 19:55

@seanbudd seanbudd left a comment

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.

I agree that it would be good to rephrase this. I've added my suggestion to break up the long sentence and reduce ambiguity. Going to await further discussion from others.

Comment thread user_docs/en/changes.t2t Outdated
Comment thread user_docs/en/changes.t2t Outdated
@XLTechie

XLTechie commented May 26, 2021 via email

Copy link
Copy Markdown
Collaborator

@seanbudd seanbudd added this to the 2021.1 milestone May 26, 2021
@XLTechie

XLTechie commented May 26, 2021 via email

Copy link
Copy Markdown
Collaborator

@seanbudd

Copy link
Copy Markdown
Member

@XLTechie @feerrenrut, my intention (and I think @CyrilleB79's) is to make the message more clear to end-users, rather than developers, that add-ons are breaking if you update. And that you need to wait for devs to fix them and release a new version.

@feerrenrut

Copy link
Copy Markdown
Contributor

The change log is also intended for addon developers. Some of which may not closely follow the development of NVDA.

I don't think it is unreasonable to provide extra guidance, especially if low cost. In other channels, this guidance seems necessary. This process is relatively new (this is only the second API compatibility change) and we don't have anything explicitly pointing authors to the change log. In other software, especially software libraries, update guides are common.

@CyrilleB79

Copy link
Copy Markdown
Contributor Author

@XLTechie wrote:

Cyrille Bougot wrote:

Please double check the following sentence: Add-ons will need to be re-tested and authors will need to provide a new version certifying its compatibility with this release.
Add-ons will need to be re-tested and authors will need to provide new versions certifying their compatibility with this release.
Do note, that the manifest language already existed in two places in changes (lines 29 and 89).

Talking about manifest in the change log is not a problem for the already two mentions:

  1. The following sentence is in the dev changes, so no problem to mention menifest:

Add-on authors are now able to provide NVDA version compatibility information in add-on manifests. (#6275, #9055)

  1. The following sentence explains the consequence of the presence of the manifest. It is in a change log item, not in the blurb. And it deals with NVDA application manifest, not add-on ones.

The NVDA application now includes a manifest, which means that it no longer runs in compatibility mode in Windows Vista.

As explained in my comment in the code, putting the word "manifest" in the blurb seems to me to make it something very common with the risk that regular users twick it to allow compatibility.

@XLTechie

XLTechie commented May 26, 2021 via email

Copy link
Copy Markdown
Collaborator

@CyrilleB79

Copy link
Copy Markdown
Contributor Author

@XLTechie wrote:

Yes, I agree with you there.

Oh sorry. I have misunderstood your previous comment.

@XLTechie

XLTechie commented May 28, 2021 via email

Copy link
Copy Markdown
Collaborator

@CyrilleB79

Copy link
Copy Markdown
Contributor Author

I guess with the downgrade of eSpeak (#12495), a new beta will be released. Could we progress this PR as well to have it merged and translated?

All NVAccess people (@Qchristensen , @seanbudd , @feerrenrut), your replies are pending on the following threads:

  • all: in the thread regarding add-on breaking release note (comment on line 14): A converging point of view among NVAccess people (or indicating who is making the final decision) is required. The pending question is if a shorter or a more detailed sentence/paragraph should be included regarding this point.
  • @Qchristensen: in the thread regarding Flash withdrawal (comment on line 11): Indicate if the initial intention was to put Flash withdrawal in the furst paragraph of the blurb or it should stand in the note with add-on breaking compa release. IMO first alternative was intended and this point can be closed without changing anything.
    Thanks.

Comment thread user_docs/en/changes.t2t
feerrenrut
feerrenrut previously approved these changes Jun 2, 2021
@feerrenrut

Copy link
Copy Markdown
Contributor

Urrgh, fixing conflicts using the web editor has changed all the line endings. I'll fix this up tomorrow.

Qchristensen
Qchristensen previously approved these changes Jun 3, 2021

@Qchristensen Qchristensen left a comment

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.

Looks good, thanks Reef!

@feerrenrut
feerrenrut merged commit 1dc486e into nvaccess:beta Jun 3, 2021
@CyrilleB79
CyrilleB79 deleted the changeLog2021.1 branch June 10, 2021 20:41
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.

5 participants