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

Add Documentation of the AppVeyor build process for NVDA forks #16293

Merged
merged 5 commits into from Apr 1, 2024

Conversation

XLTechie
Copy link
Contributor

@XLTechie XLTechie commented Mar 12, 2024

Link to issue number:

Follow-up of #16221

Summary of the issue:

The streamlined AppVeyor build process implemented in #16221, requires documentation in order to be useful.

Description of user facing changes

None.

Description of development approach

  • Wrote a document in projectDocs/dev/buildingNVDAOnAppVeyor.md.
  • Added references in readme.md and contributing.md, at @seanbudd's suggestion.
  • Took the liberty of adding a missing lead heading in buildingNVDA.md.

Testing strategy:

Checked all links to be working as of now.
Tested all commands.
I've been using variations of this process for three years.

Known issues with pull request:

None.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing

XLTechie pushed a commit to XLTechie/xlnvda that referenced this pull request Mar 12, 2024
@AppVeyorBot
Copy link

See test results for failed build of commit 845802bcb7

XLTechie pushed a commit to XLTechie/xlnvda that referenced this pull request Mar 13, 2024
@XLTechie XLTechie marked this pull request as ready for review March 13, 2024 00:55
@XLTechie XLTechie requested a review from a team as a code owner March 13, 2024 00:55
@XLTechie
Copy link
Contributor Author

CC @Qchristensen
Don't know if you also review dev docs, but if so ...

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

generally looks good. I would suggest changing Appveyor to AppVeyor across this diff

user_docs/en/changes.t2t Outdated Show resolved Hide resolved
projectDocs/dev/readme.md Outdated Show resolved Hide resolved
projectDocs/dev/contributing.md Outdated Show resolved Hide resolved
projectDocs/dev/buildingNVDAOnAppveyor.md Outdated Show resolved Hide resolved
*(FixMe: more instructions may be needed here.)*
4. Move to the "Select repository for your new project" heading, and then to the heading representing your GitHub username.
5. Below that heading, you should find your GitHub repositories listed. Go to the one representing your NVDA fork.
6. This part is less than perfectly accessible via the usual methods: you will need to use NVDA+numpad divide, to route the mouse to the repository name, and then numpad divide to click on it.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
6. This part is less than perfectly accessible via the usual methods: you will need to use NVDA+numpad divide, to route the mouse to the repository name, and then numpad divide to click on it.
6. This part is less than perfectly accessible via the usual methods: you will need to use `NVDA+numpad` divide, to route the mouse to the repository name, and then `numpadDivide` to click on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Divide" falls outside of the backticks in your suggestion. I have modified the spacing and casing to reach your intended outcome.

Suggested change
6. This part is less than perfectly accessible via the usual methods: you will need to use NVDA+numpad divide, to route the mouse to the repository name, and then numpad divide to click on it.
6. This part is less than perfectly accessible via the usual methods: you will need to use `NVDA+NumpadDivide`, to route the mouse to the repository name, and then `NumpadDivide` to click on it.

Copy link
Member

Choose a reason for hiding this comment

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

Key commands should be written in lowerCamelCase

https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/userGuideStandards.md#general-standards

The casing should be numpadDivide

projectDocs/dev/buildingNVDAOnAppveyor.md Outdated Show resolved Hide resolved
@seanbudd seanbudd marked this pull request as draft March 15, 2024 02:49
@XLTechie

This comment was marked as outdated.

@XLTechie XLTechie changed the title Add Documentation of the Appveyor build process for NVDA forks Add Documentation of the AppVeyor build process for NVDA forks Mar 15, 2024
@seanbudd seanbudd marked this pull request as ready for review March 15, 2024 05:50
@seanbudd
Copy link
Member

Just for future reference, it would have been better to keep the original commit. Commit messages don't matter much, and as it was a force push the PR has to be re-reviewed from scratch, rather than from the previous review

@XLTechie
Copy link
Contributor Author

XLTechie commented Mar 15, 2024 via email

@XLTechie
Copy link
Contributor Author

XLTechie commented Mar 15, 2024 via email

@michaelDCurran michaelDCurran merged commit f67d317 into nvaccess:master Apr 1, 2024
1 check passed
@nvaccessAuto nvaccessAuto added this to the 2024.2 milestone Apr 1, 2024
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

6 participants