Skip to content

fix: Replace broken goo.gl link with actual docs link#14081

Closed
legobeat wants to merge 5 commits intojestjs:mainfrom
legobeat:clear-link
Closed

fix: Replace broken goo.gl link with actual docs link#14081
legobeat wants to merge 5 commits intojestjs:mainfrom
legobeat:clear-link

Conversation

@legobeat
Copy link
Copy Markdown
Contributor

@legobeat legobeat commented Apr 18, 2023

Summary

  • Unobfuscate destination URL
  • Remove need for user to access Google services to discover relevant docs
  • Future-proof - no risk of sunsetting or unavailability of intermediary service causing breakage

Test plan

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Apr 18, 2023

Hi @legobeat!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Apr 18, 2023

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@legobeat legobeat marked this pull request as ready for review April 18, 2023 04:48
@legobeat legobeat mentioned this pull request Apr 18, 2023
Merged
SimenB
SimenB requested changes Apr 23, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

this is a breaking change, so cannot be done until the next major.

the URL also seems broken - it's supposed to point to https://jestjs.io/docs/snapshot-testing

@SimenB
Copy link
Copy Markdown
Member

SimenB commented Apr 23, 2023

/easycla

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Apr 23, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@SimenB SimenB removed the cla signed label Apr 23, 2023
@SimenB SimenB added this to the Jest 30 milestone Apr 23, 2023
@legobeat
Copy link
Copy Markdown
Contributor Author

legobeat commented Apr 23, 2023 •
edited
Loading

@SimenB How would this be breaking? Fixed the broken link.

@legobeat legobeat changed the title chore: Replace shortened goo.gl link with actual docs link chore: Replace broken goo.gl link with actual docs link Apr 23, 2023
@legobeat legobeat changed the title chore: Replace broken goo.gl link with actual docs link fix: Replace broken goo.gl link with actual docs link Apr 23, 2023
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 23, 2023 •
edited
Loading

✅ Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 864f06e
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/66263177f2610f0008b9b015
😎 Deploy Preview https://deploy-preview-14081--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@legobeat legobeat requested a review from SimenB April 23, 2023 12:22
@mrazauskas
Copy link
Copy Markdown
Contributor

mrazauskas commented Apr 23, 2023

How would this be breaking? Fixed the broken link.

Only in this PR almost 150 snapshots are not passing. Now imagine Renovate bot upgrading Jest: all snapshot users will get red CIs (because snapshots are not updated in CI). This simply should not happen between minor releases.

@mrazauskas
Copy link
Copy Markdown
Contributor

mrazauskas commented Apr 23, 2023

By the way, you should include change log entry too.

@legobeat
Copy link
Copy Markdown
Contributor Author

legobeat commented Apr 23, 2023

If CI indeed breaks, it would be very helpful if the workflow could be approved.

@legobeat
Copy link
Copy Markdown
Contributor Author

legobeat commented Apr 23, 2023

By the way, you should include change log entry too.

Where should a log be added, apart from bddff35?

@mrazauskas
Copy link
Copy Markdown
Contributor

mrazauskas commented Apr 23, 2023 •
edited
Loading

If CI indeed breaks, it would be very helpful if the workflow could be approved.

Do you speak about the CI in this repo? This will pass. I had in mind the CIs which run Jest for users in repos which are being updated by bots like Renovate.

@legobeat
Copy link
Copy Markdown
Contributor Author

legobeat commented Apr 23, 2023 •
edited
Loading

Hm, wouldn't the same hold f.e. for these, which were treated as non-breaking?

f2f578f (#12303)
d106643 (#14073)
e3d5491 (#10982)

@mrazauskas
Copy link
Copy Markdown
Contributor

mrazauskas commented Apr 23, 2023 •
edited
Loading

These two are changing something in this repo only (because lock file or snapshots are not shipped to NPM). The change you are making will be shipped to the users.

In other other words, the changes you are referring have zero impact on code of jest which people install from NPM, but your change will impact everyone.

mrazauskas
mrazauskas reviewed Apr 23, 2023
View reviewed changes
Comment thread packages/jest-snapshot/src/utils.ts
@legobeat legobeat requested a review from mrazauskas April 23, 2023 13:05
mrazauskas
mrazauskas reviewed Apr 23, 2023
View reviewed changes
Comment thread e2e/transform-linked-modules/ignored/symlink.js
@legobeat legobeat force-pushed the clear-link branch 2 times, most recently from c58cffa to aced412 Compare April 23, 2023 13:23
@mrazauskas
Copy link
Copy Markdown
Contributor

mrazauskas commented Apr 23, 2023

Looks good for my eye. Thanks!

@legobeat
Copy link
Copy Markdown
Contributor Author

legobeat commented Apr 29, 2023

Fixed changelog conflict.

SimenB
SimenB approved these changes May 12, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

thanks!

You don't have to keep this conflict free btw - I'll deal with it when I start landing breaking changes for v30 🙂

@legobeat legobeat requested a review from SimenB November 11, 2023 03:18
@polamjag
Copy link
Copy Markdown

polamjag commented Jul 19, 2024

Any updates? Google announced that they'll abondon all goo.gl links after August 25th, 2025: Google URL Shortener links will no longer be available - Google Developers Blog

@MillerSvt MillerSvt mentioned this pull request May 22, 2025
Closed
4 tasks
@cpojer
Copy link
Copy Markdown
Member

cpojer commented May 22, 2025

Could you rebase this PR? Happy to merge asap.

@MillerSvt MillerSvt mentioned this pull request May 22, 2025
Merged
@MillerSvt
Copy link
Copy Markdown
Contributor

MillerSvt commented May 22, 2025

@cpojer rebased

@cpojer cpojer closed this May 22, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 22, 2025

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@SimenB SimenB Awaiting requested review from SimenB

1 more reviewer

@mrazauskas mrazauskas mrazauskas left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Jest 30

Development

Successfully merging this pull request may close these issues.

7 participants

@legobeat @facebook-github-bot @SimenB @mrazauskas @polamjag @cpojer @MillerSvt