Skip to content

Upgrading Markdown and nh3 to the latest version#16724

Closed
dpy013 wants to merge 2 commits into
nvaccess:masterfrom
dpy013:Update-requirements
Closed

Upgrading Markdown and nh3 to the latest version#16724
dpy013 wants to merge 2 commits into
nvaccess:masterfrom
dpy013:Update-requirements

Conversation

@dpy013

@dpy013 dpy013 commented Jun 20, 2024 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Link to issue number:

none

Summary of the issue:

The following packages have been updated:
Markdown==3.5.1 to Markdown==3.6
nh3==0.2.15 to nh3==0.2.17

Description of development approach

Testing needs to be done with care and is recommended for review in 2024.4.

Testing strategy:

Need to test if the appveyor compilation completes successfully

Known issues with pull request:

Not yet.

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
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

Summary by CodeRabbit

  • Chores
    • Updated various dependencies to ensure compatibility and enhance security:
      • SCons updated to 4.7.0
      • comtypes updated to 1.4.4
      • requests updated to 2.32.3
      • schedule updated to 1.2.2
      • typing-extensions updated to 4.12.2
      • Pillow specification changes
      • Markdown updated to 3.6
      • nh3 updated to 0.2.17
      • sphinx updated to 7.3.7
      • sphinx_rtd_theme updated to 2.0.0

@dpy013
requirements
2e24acb 
Upgraded upgradeable packages
@dpy013 dpy013 requested a review from a team as a code owner June 20, 2024 07:06
@dpy013 dpy013 requested a review from SaschaCowley June 20, 2024 07:06
@coderabbitai

coderabbitai Bot commented Jun 20, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

This update primarily focuses on updating the versions of several Python dependencies as specified in the requirements.txt file. Key dependencies like SCons, comtypes, requests, schedule, Pillow, typing-extensions, Markdown, nh3, sphinx, and sphinx_rtd_theme have been upgraded to newer versions, along with changes in their specifications, enhancing compatibility, security, and performance.

Changes

File Change Summary
requirements.txt Updated versions for multiple dependencies including SCons (4.5.2 to 4.7.0), comtypes (1.2.0 to 1.4.4), requests (2.32.0 to 2.32.3), schedule (1.2.1 to 1.2.2), typing-extensions (4.9.0 to 4.12.2), Pillow (with specification changes), Markdown (3.5.1 to 3.6), nh3 (0.2.15 to 0.2.17), sphinx (7.2.6 to 7.3.7), and sphinx_rtd_theme (1.3.0 to 2.0.0).

Sequence Diagram(s)

Diagrams are not generated for this update as the changes are limited to version updates of dependencies and do not involve new features or modifications to the control flow.

Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e43e7c8 and a802509.

Files selected for processing (1)
  • requirements.txt (1 hunks)
Additional context used
Path-based instructions (1)
requirements.txt (1)

Pattern **/*: Focus on code smells, logic errors, edge cases, missing test cases, security flaws and serious issues. Avoid commenting on minor issues such as linting, formatting and style issues. This project uses tabs instead of spaces, do not suggest usage of spaces over tabs. Are there any 'red flags' in this code that might warrant closer investigation from a security standpoint? Explain what makes them suspicious.

Additional comments not posted (2)
requirements.txt (2)

35-35: Review the release notes of nh3 version 0.2.17 to ensure it does not introduce any breaking changes or unexpected behaviors.

Verification successful

The search for references to nh3 version 0.2.17 in the codebase only found it in the requirements.txt file, indicating that it is not widely used within the codebase. Additionally, the release notes for nh3 version 0.2.17 do not indicate any breaking changes or new features that would impact the project.

  • No additional references to nh3 version 0.2.17 were found in the codebase.
  • The release notes for nh3 version 0.2.17 primarily highlight performance improvements and do not mention any breaking changes.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any reported issues with `nh3` version 0.2.17.

# Test: Search for issues in the project's issue tracker related to `nh3`.
# Expect: No critical issues reported.
rg --type python $'nh3==0.2.17'

Length of output: 61

Script:

#!/bin/bash
# Description: Check for any reported issues with `nh3` version 0.2.17.

# Test: Search for references to `nh3` in the codebase.
rg 'nh3==0.2.17'

Length of output: 45

Script:

#!/bin/bash
# Description: Retrieve the release notes or changelog for `nh3` version 0.2.17.

# Test: Check the release notes for `nh3` version 0.2.17 for any breaking changes or new features.
curl -s https://pypi.org/pypi/nh3/0.2.17/json | jq -r '.info.description'

Length of output: 1466

30-30: Ensure compatibility with Markdown version 3.6 by reviewing the changelog and testing markdown rendering in the application.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@seanbudd seanbudd marked this pull request as draft June 20, 2024 07:20
@seanbudd

seanbudd commented Jun 20, 2024

Copy link
Copy Markdown
Member

What testing have you performed?
Have you built documentation with Sphinx?
Have you built markdown user docs?

Can you please fill out the testing section more thoroughly before marking as ready for review

@AppVeyorBot

AppVeyorBot commented Jun 20, 2024

Copy link
Copy Markdown

See test results for failed build of commit 7cf1849b80

@dpy013

dpy013 commented Jun 20, 2024

Copy link
Copy Markdown
Contributor Author

@coderabbitai fix Unit tests

@coderabbitai

coderabbitai Bot commented Jun 20, 2024

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@dp013, to assist you effectively with the failing unit tests, could you provide more details about the specific tests that are failing or share any relevant logs or error messages? This information will help in diagnosing the issue and suggesting appropriate fixes.

@seanbudd

seanbudd commented Jun 20, 2024

Copy link
Copy Markdown
Member

@dpy013 - code rabbit is a code review tool, I'd recommend checking out GitHub CoPilot if you'd like AI assistance with programming

@dpy013

dpy013 commented Jun 20, 2024

Copy link
Copy Markdown
Contributor Author

I've just done a compilation test locally and with appveyor, and the compilation was successful.
Because the first few packages have a little complexity, so I don't update it, only update the above few.

@dpy013

dpy013 commented Jun 20, 2024

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@dpy013 dpy013 changed the title Update: requirements.txt Upgrading Markdown and nh3 to the latest version Jun 20, 2024
@josephsl

josephsl commented Jun 20, 2024

Copy link
Copy Markdown
Contributor

Hi,

I'm strongly against merging this pull request as is: while updating multiple dependencies may allow updating requirements in one go, doing so is risky and can introduce problems such as dependency hell (relying on specific versions of dependencies that no longer works) and inability to isolate problems arising from one or more dependencies; next time, do one pull request per dependency. Also, as Sean pointed out, waiting for Appveyor build to complete is not a strong enough justification to say that a test was or will be performed - test with a local build first so issues can be documented as part of the pull request. Lastly, do not update dependencies simply because there are new releases available - show us evidence that you are reading release notes for these dependencies such as offering a note or two on how dependency updates can benefit NVDA project and its community.

Thanks.

@cary-rowen

cary-rowen commented Jun 20, 2024

Copy link
Copy Markdown
Contributor

I totally agree with what @josephsl said.
Please do not update these dependencies all at once, especially before you know what new changes they introduce.
If you want to contribute to NVDA there are many ways, you can also start with the documentation.
If you do want to update dependencies for NVDA, test each dependency and try to figure out as much as possible what changes were introduced in its new version. Most importantly, what positive effect does it have on NVDA or the community as a whole.

Thanks

@CyrilleB79

CyrilleB79 commented Jun 20, 2024

Copy link
Copy Markdown
Contributor

I too mainly agree with what @josephsl writes.
It's much better and easier to track and test if you update one dependency per PR.

The only little point with which I do not totally agree is to wait for a release having a visible change on NVDA to trigger an update. Updating the dependencies regularly, even if there is no direct benefit for NVDA, may be useful to capture and report any regression in the updated component. For example, that has been the case with Python update when 3.8 has been tested; it has been reverted to a bug that was reported and fixed in a later Python release (3.10 or 3.11). Testing newer releases allows to report regressions on time.

@dpy013 dpy013 mentioned this pull request Jun 21, 2024
Merged
5 tasks
@dpy013

dpy013 commented Jun 21, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for all the advice, I've read all the comments.
I'm a contributor just getting started.
Probably not very familiar with the repository yet.
Also the planning of the PRs I have submitted is very illogical.
This pr I will disassemble and select the useful and safe ones to submit.

@dpy013 dpy013 closed this Jun 21, 2024
@dpy013

dpy013 commented Jun 21, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for all the advice, I've read all the comments.
I'm a contributor just getting started.
Probably not very familiar with the repository yet.
Also the planning of the PRs I have submitted is very illogical.
This pr I will disassemble and select the useful and safe ones to submit.

seanbudd pushed a commit that referenced this pull request Jun 26, 2024
@dpy013
UpdateBuilding user documentation module: Update markdown to nh3 (#16725
922d300 
)

Follow up to #16724

Summary of the issue:
markdown 3.5.1 to 3.6
Updating python markdown to get the latest security updates
modifications

Description of user facing changes
none

Description of development approach
Updated markdown and nh3 versions of requirements.txt file
@dpy013 dpy013 deleted the Update-requirements branch June 26, 2024 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SaschaCowley SaschaCowley Awaiting requested review from SaschaCowley SaschaCowley is a code owner automatically assigned from nvaccess/developers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dpy013 @seanbudd @AppVeyorBot @josephsl @cary-rowen @CyrilleB79