Skip to content

Do not include userConfig folder in pot sources#14867

Merged
seanbudd merged 4 commits intonvaccess:masterfrom
CyrilleB79:potSources
Apr 26, 2023
Merged

Do not include userConfig folder in pot sources#14867
seanbudd merged 4 commits intonvaccess:masterfrom
CyrilleB79:potSources

Conversation

@CyrilleB79
Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #14820

Summary of the issue:

The checkPot script used to check translation comments also checks the subfolder userConfig. This is not correct since the code in userConfig is not NVDA's code but code from add-ons or from the scratchpad.

Description of user facing changes

The checkPot script does not check the userConfig subfolder anymore.

Description of development approach

  • In addition to COM interface folder, the userConfig folder is also excluded from the check.
  • Also the exclusion of these two folders is made in the folders' list rather than in the Glob expression since scons' Glob does not support filtering recursively subvolders; this is also probably more efficient to filter the folders than using Glob for each file.

Testing strategy:

Manual tests:

  • The checkPot script still find no error in NVDA's code.
  • If userConfig contains add-ons with missing translator's comments, the checkPot script does not issue errors anymore.

Unit tests: should still pass.

Known issues with pull request:

None

Change log entries:

For Developers

  • The checkPot script will not check the userConfig subfolder anymore.

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@AppVeyorBot
Copy link
Copy Markdown

See test results for failed build of commit d716c6a16a

@AppVeyorBot
Copy link
Copy Markdown

See test results for failed build of commit fd0f4373aa

@CyrilleB79
Copy link
Copy Markdown
Contributor Author

@seanbudd, I am marking this PR as ready although system tests fail, since it seems unrelated.

Note: the system test with Chrome seem to fail always more frequently. It's an issue for contributors, especially occasional ones who are not aware of the recurrent failing status of these tests.

@CyrilleB79 CyrilleB79 marked this pull request as ready for review April 24, 2023 07:33
@CyrilleB79 CyrilleB79 requested a review from a team as a code owner April 24, 2023 07:33
@CyrilleB79 CyrilleB79 requested a review from seanbudd April 24, 2023 07:33
@seanbudd
Copy link
Copy Markdown
Member

thanks @CyrilleB79

@seanbudd seanbudd merged commit a154cfa into nvaccess:master Apr 26, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Apr 26, 2023
@CyrilleB79 CyrilleB79 deleted the potSources branch April 26, 2023 15:43
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.

Ignore userConfig when running checkPot translation comments check

4 participants