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

Refactor Namespace Structure and Fix Abstractions for Lockfile and Fetcher #11528

Merged
merged 17 commits into from
Feb 11, 2025

Conversation

kbukum1
Copy link
Contributor

@kbukum1 kbukum1 commented Feb 7, 2025

What are you trying to accomplish?

This PR refactors the namespace structure and introduces necessary abstractions to improve maintainability, consistency, and clarity in Dependabot’s architecture.


🔹 Key Changes:

  • Refactored namespace structure to follow the format:
    Dependabot::{Language}::{PackageManager}
    
    • Shared logic is now under Dependabot::{Language}::Shared.
    • Package manager-specific logic is placed under Dependabot::{Language}::{PackageManager}.

📂 Updated Folder & Namespace Structure:

javascript/
├── javascript/
│   ├── bun/       → Dependabot::Javascript::Bun
│   ├── shared/    → Dependabot::Javascript::Shared
├── javascript.rb  → Dependabot::Javascript::Shared
├── bun.rb         → Dependabot::Javascript::Bun

🔹 Additional Refactors

  • Introduced LockfileBase abstraction

    • Previously, BunLock was directly referenced in shared files.
    • Now, all lockfiles extend LockfileBase, ensuring modularity and scalability.
  • Refactored Fetcher to use FileFetcherBase

    • Previously, Bun was referenced in shared fetchers.
    • Now, package managers inherit from FileFetcherBase, keeping shared code modular.

🔹 Anything you want to highlight for special attention from reviewers?

  • These changes do not introduce functional modifications but improve clarity and structure.
  • Please review:
    • Updated namespace structure for correctness and consistency.
    • New lockfile abstraction (LockfileBase) ensuring modular lockfile parsing.
    • Refactored fetcher logic to prevent package-manager-specific dependencies in shared code.

🔹 How will you know you've accomplished your goal?

  • The new namespace structure is applied consistently across all relevant files.
  • No direct references to Bun in shared fetcher or lockfile parsers.
  • All package managers use LockfileBase and FileFetcherBase abstractions.
  • All tests and linters pass successfully.
  • Codebase is more modular, making future package manager additions easier.

📌 Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@kbukum1 kbukum1 marked this pull request as ready for review February 7, 2025 22:53
@kbukum1 kbukum1 requested a review from a team as a code owner February 7, 2025 22:53
@kbukum1 kbukum1 marked this pull request as draft February 7, 2025 22:56
@kbukum1 kbukum1 force-pushed the kamil/polish_bun_ecosystem branch from 7e040d7 to b6209d2 Compare February 8, 2025 00:14
@kbukum1 kbukum1 changed the title Refactor Namespace Structure for Language and Package Manager Consistency Refactor Namespace Structure and Introduce Abstractions for Lockfile and Fetcher Feb 8, 2025
@kbukum1 kbukum1 changed the title Refactor Namespace Structure and Introduce Abstractions for Lockfile and Fetcher Refactor Namespace Structure and Fix Abstractions for Lockfile and Fetcher Feb 8, 2025
@kbukum1 kbukum1 force-pushed the kamil/polish_bun_ecosystem branch from ef609f4 to ae683da Compare February 8, 2025 00:26
@kbukum1 kbukum1 marked this pull request as ready for review February 8, 2025 00:28
@kbukum1 kbukum1 force-pushed the kamil/polish_bun_ecosystem branch from 96b557e to 30fb6c6 Compare February 8, 2025 03:38
@markhallen
Copy link
Contributor

On this branch I successfully ran:

./bin/dry-run.rb bun dsp-testing/dependabot-bun-public --updater-options=enable_bun_ecosystem --enable-beta-ecosystems.

Dry run output
   => bump @types/bun from 1.1.16 to 1.2.2

  ± bun.lock
  ~~~
  --- /tmp/original20250210-12-kmksv  2025-02-10 09:45:04.460574665 +0000
  +++ /tmp/updated20250210-12-txu0ho  2025-02-10 09:45:04.460574665 +0000
  @@ -6,7 +6,7 @@
         "dependencies": {
           "bun-types": "^1.2.0",
           "express": "^4.21.2",
  -        "lodash": "4.17.20",
  +        "lodash": "^4.17.21",
         },
         "devDependencies": {
           "@types/bun": "latest",
  @@ -17,7 +17,7 @@
       },
     },
     "packages": {
  -    "@types/bun": ["@types/[email protected]", "", { "dependencies": { "bun-types": "1.1.43" } }, "sha512-E+ue6NMcn4FXC5bDRE1W/BXUVs01h5Mt02qH8/8HGCox9akuh8KNOFdwvaQS9TDgT2RmUyJYFRRqA60WtTnm2g=="],
  +    "@types/bun": ["@types/[email protected]", "", { "dependencies": { "bun-types": "1.2.2" } }, "sha512-tr74gdku+AEDN5ergNiBnplr7hpDp3V1h7fqI2GcR/rsUaM39jpSeKH0TFibRvU0KwniRx5POgaYnaXbk0hU+w=="],
   
       "@types/node": ["@types/[email protected]", "", { "dependencies": { "undici-types": "~5.26.4" } }, "sha512-scnD59RpYD91xngrQQLGkE+6UrHUPzeKZWhhjBSa3HSkwjbQc38+q3RoIVEwxQGRw3M+j5hpNAM+lgV3cVormg=="],
   
  @@ -95,7 +95,7 @@
   
       "ipaddr.js": ["[email protected]", "", {}, "sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g=="],
   
  -    "lodash": ["[email protected]", "", {}, "sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA=="],
  +    "lodash": ["[email protected]", "", {}, "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg=="],
   
       "math-intrinsics": ["[email protected]", "", {}, "sha512-/IXtbwEk5HTPyEwyKX6hGkYXxM9nbj64B+ilVJnC/R6B0pH5G4V3b0pVbL7DBj4tkhBAppbQUlf6F6Xl9LHu1g=="],
   
  @@ -165,7 +165,7 @@
   
       "vary": ["[email protected]", "", {}, "sha512-BNGbWLfd0eUPabhkXUVm0j8uuvREyTh5ovRa/dyow/BqAbZJyC+5fU+IzQOzmAKzYqYRAISoRhdQr3eIZ/PXqg=="],
   
  -    "@types/bun/bun-types": ["[email protected]", "", { "dependencies": { "@types/node": "~20.12.8", "@types/ws": "~8.5.10" } }, "sha512-W0wCtVH+bwFp7p3Zgs03CqxEDmXxEvmmUM/FBKgWIv9T8gyeotvIjIbHzuDScc2DphhRNtr7hJLCR5PspYL5qw=="],
  +    "@types/bun/bun-types": ["[email protected]", "", { "dependencies": { "@types/node": "*", "@types/ws": "~8.5.10" } }, "sha512-RCbMH5elr9gjgDGDhkTTugA21XtJAy/9jkKe/G3WR2q17VPGhcquf9Sir6uay9iW+7P/BV0CAHA1XlHXMAVKHg=="],
   
       "send/encodeurl": ["[email protected]", "", {}, "sha512-TPJXq8JqFaVYm2CWmPvnP2Iyo4ZSM7/QKcSmuMLDObfpH5fi7RUGmd/rTDf+rut/saiDiQEeVTNgAmJEdAOx0w=="],
   
  ~~~
  5 insertions (+), 5 deletions (-)
🌍 Total requests made: '17'
  ```
  
</details>

markhallen
markhallen previously approved these changes Feb 10, 2025
@markhallen markhallen force-pushed the kamil/polish_bun_ecosystem branch 3 times, most recently from 4d499f5 to 48e7695 Compare February 10, 2025 15:16
@markhallen markhallen force-pushed the kamil/polish_bun_ecosystem branch from 48e7695 to 6277dea Compare February 10, 2025 15:23
Powe12

This comment was marked as spam.

markhallen
markhallen previously approved these changes Feb 10, 2025
@markhallen markhallen force-pushed the kamil/polish_bun_ecosystem branch from 0b29712 to 16546ef Compare February 11, 2025 10:59
@markhallen markhallen merged commit 3074681 into main Feb 11, 2025
130 of 132 checks passed
@markhallen markhallen deleted the kamil/polish_bun_ecosystem branch February 11, 2025 13:51
@kbukum1 kbukum1 self-assigned this Feb 14, 2025
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