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

Allow update of system version #148

Merged
merged 2 commits into from
Dec 16, 2022
Merged

Allow update of system version #148

merged 2 commits into from
Dec 16, 2022

Conversation

nJim
Copy link
Contributor

@nJim nJim commented Jul 15, 2022

Summary

This change allows users to update the version of their design system in the project.emulsify.json file and have the new version available to the CLI. Previously, a developer would have to delete the cached version to fetch any changes. This PR addresses #71

  • Alters the path of the cached design system to include a specific 'checkout' parameter (commit, branch, or tag of the system this project is utilizing).
  • Updates tests to reflect the changed paths. This path is stored as an md5 hash and is based on the repo URL concatinated with the checkout parameter.
  • Updates any function calling or storing design system data to use the new cache path.

Open questions:

  • Is this the best approach? The whole premise of using this md5 hash seems flawed. While this works, we are caching more than we need to. There is little reason to maintain multiple versions of a design system locally. Even if we do not want to refactor anything, we may want to consider adding a cache-clear mechanism to remove unwanted versions.
  • Will this work for non-tagged versions? This change updates a few functions that access the 'checkout' parameter in different ways. Technically this variable may be void. We may want to brainstorm some test cases before merging this work.

How to review this PR

  • Create a drupal project composer create-project drupal/recommended-project cli-update
  • Initialize a theme emulsify init "My Theme"
  • cd into the theme cd web/themes/custom/my_theme
  • Install a system emulsify system install --repository https://github.com/yalesites-org/component-library-twig.git --checkout v1.7.0
  • Verify you do not have access to the "accordion" component emulsify component list
  • Edit the theme's project.emulsify.json and change the "checkout" from v1.7.0 to v1.8.0
  • Verify you now DO have access to the accordion component emulsify component list
  • Edit the theme's project.emulsify.json and change the "checkout" from v1.8.0 back to v1.7.0
  • Verify that the accordion component is once again gone emulsify component list

Closing issues

Closes #71

@nJim nJim self-assigned this Jul 15, 2022
@nJim nJim added the 🚧 Work in Progress The PR is a work in progress. label Jul 15, 2022
@nJim nJim changed the title WIP: Allow update of system version Allow update of system version Jul 16, 2022
@nJim nJim added 👍 Ready for Review Work is ready for review. and removed 🚧 Work in Progress The PR is a work in progress. labels Jul 16, 2022
Copy link
Contributor

@mikeethedude mikeethedude left a comment

Choose a reason for hiding this comment

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

This works running through the functional testing steps. I agree that this isn't big enough and it isn't a common enough situation that this level of caching would be warranted, though I would like to ask some of our team if they are actively updated versions of systems on sites during development. I'm guessing the answer is no and maybe we don't need this cache system at all. I'm marking this approved from a perspective that it doesn't appear to break things and it could be merged as is. We may want to have a discussion about alternate options.

@mikeethedude mikeethedude added 🎉 Passes Functional Review Functionality is approved by the reviewer. 🎉 Passes Code Review Code is approved by the reviewer. and removed 👍 Ready for Review Work is ready for review. labels Aug 19, 2022
@mikeethedude mikeethedude merged commit 5f419f8 into develop Dec 16, 2022
@mikeethedude mikeethedude deleted the feat-71-cache-version branch December 16, 2022 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 Passes Code Review Code is approved by the reviewer. 🎉 Passes Functional Review Functionality is approved by the reviewer.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Can't update to a new version of a system
2 participants