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

Persists the generated Edit project in the user's .cache directory #6056

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jgoodrick
Copy link

This is a naive implementation of caching the manifests project. There may be a hashing strategy that is already in use, but I wasn't aware of it yet, if so. That could probably be improved.

In response to: #5828

Describe here the purpose of your PR.

How to test the changes locally 🧐

Include a set of steps for the reviewer to test the changes locally (see the documentation for reference).

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

…rvice and utilized it in the EditService instead of generating a temporary directory.
@pepicrft pepicrft added the changelog:added PR will be listed in the Added section of CHANGELOG label Mar 12, 2024
Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this one @jgoodrick. I left some minor comments. We can merge the PR once they are addressed.

Sources/TuistCore/Cache/CacheCategory.swift Outdated Show resolved Hide resolved
Sources/TuistKit/Services/EditService.swift Outdated Show resolved Hide resolved
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.

This will be definitely a great UX improvement 😌

Sources/TuistKit/Services/EditService.swift Outdated Show resolved Hide resolved
Sources/TuistKit/Services/EditService.swift Outdated Show resolved Hide resolved
@jgoodrick jgoodrick requested a review from pepicrft March 20, 2024 17:01
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.

Hey 👋

Sorry for not re-reviewing this PR in a timely manner. The implementation looks good but would you mind adding a couple of unit tests? A new behavior without any raises a red flag 😁

Copy link
Member

Choose a reason for hiding this comment

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

I know we currently don't have tests for this service but can we add some?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:added PR will be listed in the Added section of CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants