Automattic / Automattic/dangermattic

Reconsider manifest check logic for Bundler and SPM

Open
#45 1 comment 1 reaction 0 assignees View on GitHub
Dominant language
Ruby
Stars
10
Forks
2
PR merge metrics
No merged PRs in 30d

Description

Below is a verbatim copy of my comment on WordPress iOS, https://github.com/wordpress-mobile/WordPress-iOS/pull/22525#issuecomment-1928592156, describing the issue.

---

`@iangmaia` I believe the Danger comment (the 2nd version, in case it changes in the future after new commits), https://github.com/wordpress-mobile/WordPress-iOS/pull/22525#issuecomment-1922301618 ,about the `Package.resolved` is out of place 🤔

![image](https://github.com/wordpress-mobile/WordPress-iOS/assets/1218433/9e5ea74e-38e3-43c1-a9ef-59f94ae6ab20)

I think we might have been too optimistic in implementing the `Package.swift` `Package.resolve` check. 🤔 CocoaPods tracks an hash of the `Podfile` content in `Podfile.lock`, so any change to `Podfile` generates a changes in `Podfile.lock`. But Bundler and SPM don't do this.

I don't know how to go about "fixing" the behavior, because one would have to parse the change itself. E.g. if `Gemfile` changed with only comments, do not expect `Gemfile.lock` to change. If a new gem was added, expect it to change.

To make matter worse... The modules package currently defines no dependencies and therefore has no matching `Package.resolved`!

One idea to start with would be to simply relax the warning. Something like, from:

> `Modules/Package.swift` was changed without updating its corresponding `Package.resolved`. Please resolve the Swift packages in Xcode.

to:

> `Modules/Package.swift` was changed but its corresponding `Package.resolved` was not updated. Make sure you resolved the Swift packages in Xcode. If you have done so already, you can disregard this warning.

Contributor guide

No contributing guide indexed for this repository

Research direction

Review the existing manifest checks involving Gemfile/Gemfile.lock and Package.swift/Package.resolved, then use the linked WordPress iOS comment for the reported behavior. Done means the Bundler and SPM warnings no longer assume every manifest change requires a lockfile update, while still communicating what contributors should verify.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby, swift
Domain
ci-cd, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.