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

docs: update readme on arm64 images #2300

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Conversation

slhck
Copy link
Contributor

@slhck slhck commented Jul 9, 2024

User description

Description

  • Fix description for ARM64 images, mention the important information about usage and browser support in a dedicated table.
  • Add a reference to the related issue.
  • Move the history of the (now archived) fork below, since it is mostly irrelevant for end-users.
  • Also move the manual build documentation below, because the prebuilt images should be used by end-users.
  • Fix some minor typos.

Motivation and Context

From #2298 it was not clear that running AMD64 images under Apple Silicon was not supported.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes. -> N/A
  • All new and existing tests passed. -> N/A

PR Type

Documentation


Description

  • Updated the README to provide a clearer description of ARM64 images and their usage.
  • Added a dedicated table to detail browser support across different architectures.
  • Moved historical information about the archived fork and manual build instructions to lower sections for better readability.
  • Fixed minor typos and improved the overall clarity of the documentation.

Changes walkthrough 📝

Relevant files
Documentation
README.md
Update README for ARM64 images and browser support             

README.md

  • Updated description for ARM64 images.
  • Added a table for browser support in multi-arch images.
  • Moved historical and manual build information to lower sections.
  • Fixed minor typos and improved clarity.
  • +44/-33 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add a brief explanation for the make set_containerd_image_store command to provide more context

    Consider adding a brief explanation about the purpose of the make
    set_containerd_image_store command to provide more context for users who may not be
    familiar with it.

    README.md [173-175]

     ```bash
    +# This command enables the containerd image store feature in Docker Engine
     make set_containerd_image_store
    
    - [ ] **Apply this suggestion** <!-- /improve --apply_suggestion=0 -->
    
    <details><summary>Suggestion importance[1-10]: 7</summary>
    
    Why: Adding a comment to explain the command in the code block enhances understanding for users unfamiliar with it, improving documentation quality.
    
    
    </details></details></td><td align=center>7
    
    </td></tr><tr><td>
    
    
    
    <details><summary>Add an explanation about the CircleCI status badge to inform users what it represents</summary>
    
    ___
    
    
    **Consider adding a brief explanation about the significance of the CircleCI status badge to <br>inform users what it represents.**
        
    [README.md [152-153]](https://github.com/SeleniumHQ/docker-selenium/pull/2300/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R152-R153)
    
    ```diff
     [![CircleCI](https://dl.circleci.com/status-badge/img/gh/SeleniumHQ/docker-selenium/tree/trunk.svg?style=svg)](https://dl.circleci.com/status-badge/redirect/gh/SeleniumHQ/docker-selenium/tree/trunk)
    +This badge indicates the current build status of the project on CircleCI.
     
    
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Providing context about the CircleCI status badge helps users understand its significance, enhancing the documentation's informativeness.

    7
    Readability
    Rephrase the note about Firefox availability for better readability

    To improve readability, consider rephrasing the note about Firefox availability to make it
    clearer that the ARM64 version will differ from the AMD64 version until the stable
    release.

    README.md [148-149]

    -The Firefox version in ARM64 will be different with the AMD64 until the stable release is available. The Firefox (node and standalone) images are available in multi-arch.
    +The Firefox version for ARM64 will differ from the AMD64 version until the stable release is available. The Firefox (node and standalone) images are available in multi-arch.
     
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggested rephrasing slightly improves readability but does not significantly impact the overall understanding or accuracy of the information.

    5
    Rephrase the note about running AMD64 images under emulation on ARM64 platforms for better clarity

    To improve clarity, consider rephrasing the note about running AMD64 images under
    emulation on ARM64 platforms.

    README.md [139]

    -Running an AMD64 image under emulation on an ARM64 platform is not recommended due to performance and [stability issues](https://github.com/SeleniumHQ/docker-selenium/issues/2298).
    +Running AMD64 images under emulation on ARM64 platforms is not recommended due to performance and [stability issues](https://github.com/SeleniumHQ/docker-selenium/issues/2298).
     
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The rephrasing slightly improves clarity but does not significantly change the original meaning or impact of the statement.

    5

    @VietND96 VietND96 merged commit 4d4d286 into SeleniumHQ:trunk Jul 9, 2024
    16 checks passed
    @VietND96
    Copy link
    Member

    VietND96 commented Jul 9, 2024

    Thank you, @slhck !

    @slhck slhck deleted the update-readme branch July 10, 2024 04:31
    @VietND96 VietND96 added this to the 4.23 milestone Jul 19, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants