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

Change ProjectDescription model constructors to static functions #5843

Merged
merged 12 commits into from
Jan 29, 2024
Merged

Change ProjectDescription model constructors to static functions #5843

merged 12 commits into from
Jan 29, 2024

Conversation

fortmarek
Copy link
Member

@fortmarek fortmarek commented Jan 26, 2024 •
edited

Resolves #5714

Short description 📝

We're making all constructors of ProjectDescription models internal in favor of public static constructors with the exception of:

  • Top-level models like Project (any that needs to run dumpIfNeeded) -> this convention is inline with PackageDescription and imho makes sense

How to test the changes locally 🧐

CI should pass

Contributor checklist ✅

  • The code has been linted using run mise run lint:fix
  • The change is tested via unit testing or acceptance testing, or both
  • The title of the PR is formulated in a way that is usable as a changelog entry
  • In case the PR introduces changes that affect users, the documentation has been updated

Reviewer checklist ✅

  • The code architecture and patterns are consistent with the rest of the codebase
  • Reviewer has checked that, if needed, the PR includes the label changelog:added, changelog:fixed, or changelog:changed, and the title is usable as a changelog entry

@fortmarek fortmarek changed the base branch from main to tuist-4 January 26, 2024 11:08
danieleformichelli
danieleformichelli approved these changes Jan 26, 2024
View reviewed changes
Sources/ProjectDescription/Arguments.swift Outdated
@@ -4,17 +4,11 @@ import Foundation
public struct Arguments: Equatable, Codable {
public var environmentVariables: [String: EnvironmentVariable]
public var launchArguments: [LaunchArgument]

public init(

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Sources/ProjectDescription/EnvironmentVariable.swift Outdated
Comment on lines 12 to 17
// MARK: - Init

public init(value: String, isEnabled: Bool) {
init(value: String, isEnabled: Bool) {
self.value = value
self.isEnabled = isEnabled
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just remove this?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, if you define an init(stringLiteral:), you will lose the init generated by the compiler

Copy link
Collaborator

Choose a reason for hiding this comment

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

You are right! No strong opinion, I would define that in an extension, so we have the generated one for free.

@fortmarek fortmarek force-pushed the project-description/static-inits branch from e73cba9 to 927331a Compare January 26, 2024 14:06
@fortmarek fortmarek added the changelog:changed PR will be listed in the Changed section of CHANGELOG label Jan 26, 2024
@fortmarek fortmarek marked this pull request as ready for review January 26, 2024 14:16
@fortmarek fortmarek changed the title Project description/static inits Change ProjectDescription model constructors to static functions Jan 26, 2024
@fortmarek fortmarek mentioned this pull request Jan 26, 2024
Merged
20 tasks
@danieleformichelli danieleformichelli merged commit 2caccd4 into tuist-4 Jan 29, 2024
10 checks passed
@danieleformichelli danieleformichelli deleted the project-description/static-inits branch January 29, 2024 10:33
@fortmarek fortmarek mentioned this pull request Jan 29, 2024
Closed
@waltflanagan waltflanagan mentioned this pull request Jan 30, 2024
Closed
6 tasks
pepicrft added a commit that referenced this pull request Jan 31, 2024
@pepicrft @danieleformichelli
@dxmvsh @fortmarek @cpisciotta @waltflanagan
Tuist 4 (#5567)
a61e6e5 
* Remove tuist-env

* Fix the release pipeline

* Remove deprecated methods (#5560)

* [tuist-4] Remove `tuistenv` (#5556)

* Remove tuist-env

* Fix the release pipeline

* chore: remove deprecated methods

* chore: remove disableAnalytics

* fix: fix fixture definition

---------

Co-authored-by: Pedro Piñera Buendía <663605+pepicrft@users.noreply.github.com>

* Fix the bundle script

* Change the coloring behaviour (#5668)

* Not auto-generate a scheme per platform (#5679)

* Eliminate signing capabilities (#5716)

* Eliminate signing capabilities

* Update the root Package.resolved

* Removed TargetDependency's .packagePlugin API in favor of using PackageType (#5719)

* feat(TargetDependency): remove packagePlugin api

* feat(TargetDependency+ManifestMapper): remove packagePlugin api handle

* test(TargetDependency+ManifestMapperTests): add tests for package types: runtime, macro and plugin

* style(TargetDependency+ManifestMapperTests): add lint changes

* chore: remove carthage support (#5740)

* Resolve Tuist-4 Compiler Errors (#5818)

* Remove Dependencies.swift (#5812)

* Squashed commit of the following:

commit f74176d
Author: fortmarek <marekfort@me.com>
Date:   Tue Jan 16 09:21:28 2024 +0100

    Remove cache warm for Silicon

commit b954bdb
Author: fortmarek <marekfort@me.com>
Date:   Tue Jan 16 09:20:46 2024 +0100

    Minor updates

commit 4693f71
Author: fortmarek <marekfort@me.com>
Date:   Mon Jan 15 23:52:01 2024 +0100

    Run lint:fix

commit 984a73f
Author: fortmarek <marekfort@me.com>
Date:   Mon Jan 15 23:51:44 2024 +0100

    Update external dependencies tutorial

commit 909fc24
Author: fortmarek <marekfort@me.com>
Date:   Mon Jan 15 23:45:31 2024 +0100

    Add PackageSettings in Package.swift

commit c70affb
Merge: a932745 7f34cba
Author: fortmarek <marekfort@me.com>
Date:   Mon Jan 15 09:35:51 2024 +0100

    Merge remote-tracking branch 'origin/main' into tests/dependencies-acceptance

commit a932745
Merge: ef80095 675239c
Author: Daniele Formichelli <df@bendingspoons.com>
Date:   Tue Jan 2 13:55:15 2024 +0100

    Merge branch 'main' into tests/dependencies-acceptance

commit ef80095
Merge: 54f9875 e14da60
Author: Daniele Formichelli <df@bendingspoons.com>
Date:   Sun Dec 31 16:55:49 2023 +0100

    Merge branch 'main' into tests/dependencies-acceptance

commit 54f9875
Author: Daniele Formichelli <df@bendingspoons.com>
Date:   Sun Dec 31 16:55:43 2023 +0100

    fix: feature one target sources

commit 9671063
Author: Daniele Formichelli <df@bendingspoons.com>
Date:   Sat Dec 30 16:52:44 2023 +0100

    chore: format

commit 7df88d4
Author: Daniele Formichelli <df@bendingspoons.com>
Date:   Sat Dec 30 16:46:10 2023 +0100

    chore: simplify spm fixture

commit 9a05c9e
Author: Daniele Formichelli <df@bendingspoons.com>
Date:   Sat Dec 30 15:47:38 2023 +0100

    chore: lint

commit 96c989f
Author: Daniele Formichelli <df@bendingspoons.com>
Date:   Fri Dec 29 21:34:45 2023 +0100

    test: re-enable dependencies acceptance tests

* Remove Dependencies.swift support

* Remove intermediary graph.json

* Fix issues after merging tuist-4

* Remove Dependencies.swift from fixtures

* Move Package loading to TuistLoader

* Remove TuistDependenciesTesting from FetchServiceTests

* Add dump command for package

* Add back PackageSettings in app_with_spm_dependencies

* Fix missing tests due to missing Package.swift error

* Remove platforms from string assertion due to Set non-determinism

* Add package settings to multiplatform_app_with_sdk fixture

* Remove ProjectDescriptionHelpers import in Package.swift for multiplatform_app_with_sdk

* Update platforms for multiplatform_app_with_sdk fixture

* Remove deprecated APIs

* Fix Test Compilation

* Update test fixture tempalte to multiplatform API

* Migrate fixtures to `destinations` api

* Migrate fixtures to `deploymentTargets` apis

* Update manifest helper methods

* Update fixture generator to use `destinations`

* Update to use a tuist 4 version of this template repo

* Fixing a couple fixtures

* Fix Tuist helper method to use correct signature

* Update doc comments

* More missed callsites

* Remove Cache from ProjectDescription (#5833)

* Fix missing macro case in switch

* Remove binary cache-related models from TuistCore

* chore: fix warnings

* Change ProjectDescription model constructors to static functions (#5843)

* Remove public inits in favor of static constructors

* Fix tests

* Lint

* Fix remaining tests

* Skip loading default helpers if equal to srcroot

* Fix lint issue

* Fix infinite loop in app_with_spm_dependencies fixture

* Lint

* Remove unnecessary force try

* Fix generate acceptance tests

* Fix test acceptance tests

* Rename cache directory name (#5842)

* Rename cache directory name

* Rename cache categories

* Fix compilation issues

* Fix CleanServiceTests

---------

Co-authored-by: Daniele Formichelli <df@bendingspoons.com>

* Bump Minimum Swift to 5.9

* Autoformat code style

- Update .swiftformat
- Ignore unwanted rules

* Fix failing tests

* Simplify tuist init (#5853)

* Simplify tuist init

* Address PR feedback

* Add Package.swift, update init message

* Update tabs

* Remove Package.swift

---------

Co-authored-by: Daniele Formichelli <df@bendingspoons.com>
Co-authored-by: Dimash <44325936+dxmvsh@users.noreply.github.com>
Co-authored-by: fortmarek <marekfort@me.com>
Co-authored-by: Charles Pisciotta <cpisciottadeveloping@gmail.com>
Co-authored-by: Mike Simons <msimons@etsy.com>
Co-authored-by: Mike Simons <waltflanagan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danieleformichelli danieleformichelli danieleformichelli approved these changes

@pepicrft pepicrft Awaiting requested review from pepicrft

Assignees
No one assigned
Labels
changelog:changed PR will be listed in the Changed section of CHANGELOG
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@fortmarek @danieleformichelli