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.

Sign up for GitHub

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

Jump to bottom

Edit Dangerfile as a Swift Package on danger-swift edit instead of xcodeproj #566

Merged
merged 15 commits into from
Apr 23, 2023
Merged

Edit Dangerfile as a Swift Package on danger-swift edit instead of xcodeproj #566

merged 15 commits into from
Apr 23, 2023

Conversation

417-72KI
Copy link
Member

Resolves: #563

417-72KI
417-72KI commented Jan 25, 2023
View reviewed changes
Sources/DangerDependenciesResolver/Script.swift
@@ -163,7 +163,7 @@ public final class Script {
do {
let path = editingPath()

try ShellExecutor().spawn("open \"\(path)\"", arguments: [])
try ShellExecutor().spawn("xed \"\(path)\"", arguments: [])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About xed
https://dive.medium.com/xcode-invocation-tool-xed-deb04d03cf98

@417-72KI 417-72KI force-pushed the edit-as-package branch 4 times, most recently from 10b1fa3 to d57b402 Compare January 26, 2023 14:20
417-72KI
417-72KI commented Jan 26, 2023
View reviewed changes
Sources/Runner/Commands/Edit.swift

try createConfig(atPath: configPath, libPath: absoluteLibPath, libsImport: libsImport)

try script.setupForEdit(importedFiles: importedFiles, configPath: configPath)
Copy link
Member Author

@417-72KI 417-72KI Jan 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No flags can be set on Package.swift, and needs Danger package as a default dependency instead.
https://github.com/danger/swift/pull/566/files?w=1#diff-b040c9d7cf1d8a4d0275190fb078df8712464dcb18a89b281f9a93145610068dR16

This may resolves #464 only on danger-swift edit.

@417-72KI 417-72KI changed the title [WIP] Edit Dangerfile as a Swift Package on danger-swift edit instead of xcodeproj Edit Dangerfile as a Swift Package on danger-swift edit instead of xcodeproj Mar 3, 2023
@417-72KI 417-72KI marked this pull request as ready for review March 3, 2023 15:57
f-meloni
f-meloni reviewed Mar 31, 2023
View reviewed changes
Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a quick look and looks great!
I will give it a more focused look and merge it!
Thank you for this amazing PR :)

f-meloni
f-meloni reviewed Apr 4, 2023
View reviewed changes
Sources/DangerDependenciesResolver/InlineDependenciesFinder.swift Outdated

extension InlineDependenciesFinder.InlineDependency {
static var dangerSwift: Self {
.init(url: URL(string: "https://github.com/danger/swift.git")!,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be depending on the latest version, which might differ from your current version, and that might lead to mistakes, am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.
There is a DangerVersion constant in main.swift, and I will try if I can inject it.

Copy link
Member Author

@417-72KI 417-72KI Apr 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something like this?
22b5b6b...706164c

@417-72KI 417-72KI force-pushed the edit-as-package branch 2 times, most recently from c68e9ee to 706164c Compare April 4, 2023 19:05
@orta orta mentioned this pull request Apr 5, 2023
Open
@417-72KI
Copy link
Member Author

417-72KI commented Apr 6, 2023

NOTE:
CI on Swift 5.7 fails due to SwiftLint issue (realm/SwiftLint#4031).

I tried to bump SwiftLint to resolved version, error occured on resolving dependencies.

Failed to resolve dependencies Dependencies could not be resolved because root depends on 'swiftlint' 0.51.0..<1.0.0 and root depends on 'rocket' 1.2.1..<2.0.0.
'rocket' is incompatible with 'swiftlint' because 'swiftlint' 0.51.0 depends on 'yams' 5.0.5..<6.0.0 and no versions of 'swiftlint' match the requirement 0.51.1..<1.0.0.
'rocket' >= 1.2.1 practically depends on 'yams' 4.0.0..<5.0.0 because no versions of 'rocket' match the requirement 1.2.2..<2.0.0 and 'rocket' 1.2.1 depends on 'yams' 4.0.0..<5.0.0.

@417-72KI 417-72KI mentioned this pull request Apr 6, 2023
Merged
@417-72KI
Copy link
Member Author

417-72KI commented Apr 8, 2023

@f-meloni @orta
Could you please review shibapm/Rocket#50 and release it if you can?
It is a bottleneck for this.

@417-72KI 417-72KI force-pushed the edit-as-package branch 3 times, most recently from ebedf08 to a5c195b Compare April 20, 2023 15:37
@f-meloni f-meloni merged commit 3022cf9 into danger:master Apr 23, 2023
15 checks passed
@417-72KI 417-72KI deleted the edit-as-package branch April 24, 2023 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@f-meloni f-meloni f-meloni approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

'generate-xcodeproj' is no longer needed and will be deprecated soon
2 participants
@417-72KI @f-meloni