-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
postgresql: Update manifest #7156
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
base: master
Are you sure you want to change the base?
Conversation
- Make `notes` neater and clearer - Remove `depends` , files can now be extracted correctly without `7zip19.00-helper`
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
WalkthroughUpdated Scoop manifest for PostgreSQL: removed depends, replaced pre_install with extract_dir="pgsql", simplified notes, and adjusted 64-bit URLs (and autoupdate URLs) to remove URL fragments. Other fields remain unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip You can disable poems in the walkthrough.Disable the Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Your changes do not pass checks. postgresql
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
bucket/postgresql.json (1)
7-8
: Notes copy tidy-up looks good; consider being explicit with PGDATA.To avoid ambiguity if PGDATA isn’t set in a custom shell/session, you could show the explicit flag in the examples.
- "Run 'pg_ctl start' to start the database and 'pg_ctl stop' to stop it.", + "Run 'pg_ctl -D \"%PGDATA%\" start' to start the database and 'pg_ctl -D \"%PGDATA%\" stop' to stop it.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/postgresql.json
(2 hunks)
🔇 Additional comments (3)
bucket/postgresql.json (3)
13-13
: URL fragment removal LGTM; revalidate the hash.Dropping
#/dl.zip_
is correct and should allow Scoop to auto-extract. Since the file bytes are the same, the hash likely remains valid—please recheck locally to be safe.
17-17
: Confirm archive layout for extract_dir=pgsql.This assumes the zip’s top-level directory is consistently
pgsql
. If EDB ever changes this, installs will fail. Please confirm for 17.4 and spot-check a prior version.
43-43
: Autoupdate URL aligned with runtime URL.Looks correct and consistent with
checkver
. No further action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
bucket/postgresql.json (3)
2-2
: Consider including the packaging revision in version (to avoid hardcoding “-1” elsewhere).EDB filenames include a “-” suffix. Keeping
"version": "17.6"
while hardcoding-1
in URLs risks breakage if a-2
re-spin appears. Easiest alignment: encode the revision inversion
now.Apply:
- "version": "17.6", + "version": "17.6-1",Follow-up: see my suggestions on checkver and autoupdate below to keep things consistent.
36-39
: Optional: make checkver selection safer and (if you encode revision in version) format it via replace.
- Add
"reverse": true
to prefer the latest directory if listing order varies.- If you adopt
version: "17.6-1"
, you can stamp-1
viareplace
for now."checkver": { "url": "https://www.postgresql.org/ftp/source/", - "regex": "v(?<version>[\\d.]+)/" + "regex": "v(?<version>[\\d.]+)/", + "reverse": true, + "replace": "$version-1" },Note: This still won’t auto-track a future
-2
re-spin, but it keeps manifest fields consistent and removes the need to hardcode-1
elsewhere.
43-43
: Autoupdate URL: avoid hardcoding “-1” if you move the revision into version.With
version: "17.6-1"
, let the URL consume$version
directly.- "url": "https://get.enterprisedb.com/postgresql/postgresql-$version-1-windows-x64-binaries.zip" + "url": "https://get.enterprisedb.com/postgresql/postgresql-$version-windows-x64-binaries.zip"If you keep
version: "17.6"
, current line is fine—just be aware it will miss-2
re-spins.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/postgresql.json
(2 hunks)
🔇 Additional comments (3)
bucket/postgresql.json (3)
7-8
: Notes copyedits look good.Clearer phrasing; matches PGDATA usage set by env.
17-17
: extract_dir=“pgsql” is the right simplification; please double-check the top-level folder.Most EDB binaries zips root under “pgsql/”. The script above also validates this implicitly via pg_ctl.exe presence.
13-14
: Verified — SHA256 and ZIP layout OK.SHA256 matches d378882abd001a186735acd6f6ba716bca6ccd192e800412d4fd15ed25376b3e; archive contains pgsql/ and pgsql/bin/pg_ctl.exe (pgAdmin 4 present).
/verify |
All changes look good. Wait for review from human collaborators. postgresql
|
notes
neater and clearerdepends
, files can now be extracted correctly without7zip19.00-helper
(refer to [email protected]: decompress error #3816)checkver
, add/
at its endversion
to 17.6 manually<manifest-name[@version]|chore>: <general summary of the pull request>
Summary by CodeRabbit
Bug Fixes
Documentation
Chores