Skip to content

Conversation

@voxik
Copy link

@voxik voxik commented Dec 23, 2024

User description

This directive has no effect without specifying executables. And the shipped executables do not qualify to be listed, because they are not "executable Ruby files" [1]. Moreover they are located in nested directories.


PR Type

Enhancement


Description

  • Removed the bindir directive from selenium-webdriver gemspec since it has no effect without specifying executables
  • The shipped executables do not qualify as "executable Ruby files" and are located in nested directories, making the directive unnecessary
  • Improves gemspec clarity by removing unused configuration

Changes walkthrough 📝

Relevant files
Configuration changes
selenium-webdriver.gemspec
Remove unused bindir directive from gemspec configuration

rb/selenium-webdriver.gemspec

  • Removed bindir directive from gemspec as it has no effect without
    executables
  • Cleaned up unnecessary configuration since executables are in nested
    directories
  • +0/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    This directive has no effect without specifying `executables`. And the
    shipped executables do not qualify to be listed, because they are not
    "executable Ruby files" [[1]]. Moreover they are located in nested
    directories.
    
    [1]: https://github.com/rubygems/guides/blob/4357756b8cf9bef92b90e717306b5bf9c332d9f6/specification-reference.md?plain=1#L387
    @CLAassistant
    Copy link

    CLAassistant commented Dec 23, 2024

    CLA assistant check
    All committers have signed the CLA.

    @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    @qodo-merge-pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @voxik
    Copy link
    Author

    voxik commented Dec 23, 2024

    Just FTR, I don't think I am allowed to sign CLA, therefore do whatever you have to do with this contribution ...

    @diemol diemol requested review from aguspe and p0deje December 23, 2024 13:12
    @p0deje
    Copy link
    Member

    p0deje commented Dec 23, 2024

    We can't merge it without CLA, so I'll have to close this PR. I agree that in current form bindir is not useful, but AFAIK it's harmless too, so we can keep it.

    @p0deje p0deje closed this Dec 23, 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.

    3 participants