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

BREAKING CHANGE: Remove dependency twig/twig for v1 #1396

Merged
merged 2 commits into from
May 9, 2024

Conversation

curdaj
Copy link
Contributor

@curdaj curdaj commented May 3, 2024

Description

Additional context

Issue reference

Copy link

netlify bot commented May 3, 2024

Deploy Preview for spirit-design-system ready!

Name Link
🔨 Latest commit 9465aa3
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system/deploys/663c99081c6e320008a7d477
😎 Deploy Preview https://deploy-preview-1396--spirit-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 96 (no change from production)
Accessibility: 93 (no change from production)
Best Practices: 100 (no change from production)
SEO: 82 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@coveralls
Copy link

coveralls commented May 3, 2024

Coverage Status

coverage: 96.371% (+16.0%) from 80.418%
when pulling 5dde7f8 on deps/ds-975-remove-support-twig-v1
into a3bfba3 on integration/release-v2.

@curdaj curdaj marked this pull request as ready for review May 3, 2024 12:12
@literat
Copy link
Collaborator

literat commented May 6, 2024

Just to be sure: no updated composer.lock? Even in the Twig demo app? Did you try to run composer update? This does not mean that this PR is not correct, but I am trying to verify the change. Thanks.

@curdaj curdaj force-pushed the deps/ds-975-remove-support-twig-v1 branch from 1f9df36 to 4293551 Compare May 6, 2024 11:50
@curdaj curdaj changed the title Deps: Remove dependency twig/twig for v1 BREAKING CHANGE: Remove dependency twig/twig for v1 May 6, 2024
@crishpeen
Copy link
Member

crishpeen commented May 7, 2024

Just to be sure: no updated composer.lock? Even in the Twig demo app? Did you try to run composer update? This does not mean that this PR is not correct, but I am trying to verify the change. Thanks.

composer update updated all dependencies, is it ok @literat? They are still in allowed range set in composer.json, but I am not sure if we want to do that.

EDIT: I checked with Tom and it is ok, as the lock is updated only in our demo and it works. So LGTM!

@crishpeen
Copy link
Member

Can you please change the first commit message to

BREAKING CHANGE(web-twig): Remove dependency twig/twig for v1

@literat
Copy link
Collaborator

literat commented May 7, 2024

Will be mergeable after this PR: #1404. It looks like we are not ignoring lockfiles and the Composer changed the formatting of the lockfile.

@literat
Copy link
Collaborator

literat commented May 9, 2024

Will be mergeable after this PR: #1404. It looks like we are not ignoring lockfiles and the Composer changed the formatting of the lockfile.

This has been merged into the main. Please, rebase the entire release and this branch also.

@curdaj curdaj force-pushed the deps/ds-975-remove-support-twig-v1 branch from 5dde7f8 to 32904d1 Compare May 9, 2024 09:29
@curdaj curdaj force-pushed the integration/release-v2 branch from 6212b9d to 6bafe41 Compare May 9, 2024 09:31
@curdaj curdaj force-pushed the deps/ds-975-remove-support-twig-v1 branch from 32904d1 to 9465aa3 Compare May 9, 2024 09:36
@curdaj curdaj merged commit 031ced4 into integration/release-v2 May 9, 2024
27 checks passed
@curdaj curdaj deleted the deps/ds-975-remove-support-twig-v1 branch May 9, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants