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

fix: replace TargetMetadata.isLocal with Target.type enum #91

Merged
merged 3 commits into from
Dec 26, 2024

Conversation

pepicrft
Copy link
Contributor

@pepicrft pepicrft commented Dec 26, 2024

Replaces the isLocal boolean property with a new TargetType enum that better represents whether a target is local or remote. This change provides more explicit semantics around target types and removes the need for documentation comments explaining the boolean's meaning.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pepicrft pepicrft requested a review from fortmarek December 26, 2024 12:46
@pepicrft pepicrft added the enhancement New feature or request label Dec 26, 2024 — with Graphite App
@pepicrft pepicrft marked this pull request as ready for review December 26, 2024 12:46
@pepicrft pepicrft changed the title Turn isLocal into Target.type fix: replace TargetMetadata.isLocal with Target.type enum Dec 26, 2024
@pepicrft pepicrft force-pushed the 12-26-turn_islocal_into_target.type branch from 4be3c2d to 1efa61c Compare December 26, 2024 13:33
@pepicrft pepicrft force-pushed the 12-26-turn_islocal_into_target.type branch from 1efa61c to 850d308 Compare December 26, 2024 13:34
Copy link
Member

@fortmarek fortmarek left a comment

Choose a reason for hiding this comment

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

Thanks!

}

init(tags: Set<String>, isLocal: Bool) {
init(tags: Set<String>, isLocal _: Bool) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
init(tags: Set<String>, isLocal _: Bool) {
init(tags: Set<String>) {

@@ -0,0 +1,6 @@
public enum TargetType: Codable, Hashable, Equatable, Sendable {
/// A target is local when it hasn't been resolved and pulled by a package manager (e.g., SPM).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// A target is local when it hasn't been resolved and pulled by a package manager (e.g., SPM).
/// A target is local when it hasn't been resolved and pulled by a package manager (e.g., SwiftPM).

@pepicrft pepicrft self-assigned this Dec 26, 2024
Copy link
Contributor Author

pepicrft commented Dec 26, 2024

Merge activity

  • Dec 26, 9:21 AM EST: A user started a stack merge that includes this pull request via Graphite.
  • Dec 26, 9:21 AM EST: A user merged this pull request with Graphite.

@pepicrft pepicrft merged commit c11bcd2 into main Dec 26, 2024
19 checks passed
@pepicrft pepicrft deleted the 12-26-turn_islocal_into_target.type branch December 26, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants