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

chore: make spec migration screen text more generic #22325

Merged
merged 2 commits into from Jun 16, 2022
Merged

Conversation

lmiller1990
Copy link
Contributor

@lmiller1990 lmiller1990 commented Jun 15, 2022

User facing changelog

N/A

Additional details

Make the text more generic, as to cover all the cases where we automatically change specs:

  • rename
  • move
  • rename and move

Steps to test

Just open launchpad to any Cypress 9 project and have a look.

How has the user experience changed?

Improved language in migration UI to be more clear about intended and recommended behavior.

Before

image

After

Screen Shot 2022-06-16 at 12 14 58 am

PR Tasks

  • [na] Have tests been added/updated? It's just a json file, so no, there isn't any to update
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only) YES 10.1.1
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 15, 2022

Thanks for taking the time to open a PR!

@lmiller1990 lmiller1990 marked this pull request as ready for review June 15, 2022 02:45
@cypress
Copy link

cypress bot commented Jun 15, 2022



Test summary

37553 0 454 0Flakiness 4


Run details

Project cypress
Status Passed
Commit fe93821
Started Jun 15, 2022 2:23 PM
Ended Jun 15, 2022 2:39 PM
Duration 16:10 💡
OS Linux Debian - 10.11
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

commands/actions/click.cy.js Flakiness
1 ... > scroll-behavior > can scroll to and click elements in html with scroll-behavior: smooth
next.cy.ts Flakiness
1 Working with next-12.1.6 > should detect new spec
cypress/proxy-logging.cy.ts Flakiness
1 Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second
2 Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@lmiller1990 lmiller1990 self-assigned this Jun 15, 2022
Copy link
Contributor

@marktnoonan marktnoonan left a comment

Choose a reason for hiding this comment

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

"Updating specs" describes something potentially much broader than renaming and moving. I've suggested a smaller change that might still be generic enough.

@@ -543,7 +543,7 @@
"after": "After",
"heresWhy": "here's why:",
"renameAuto": {
"title": "We recommend automatically renaming your specs in this step",
"title": "We recommend automatically updating your specs in this step",
Copy link
Contributor

Choose a reason for hiding this comment

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

"updating" sounds to broad to me, what we recommend is definitely renaming. This title probably doesn't need a change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, reverted

@@ -598,7 +598,7 @@
"description": "Complete the steps below to migrate your project to Cypress 10",
"step1": {
"title": "Rename existing specs",
"description": "In this step, we'll automatically rename and move your existing spec files.",
"description": "In this step, we'll automatically change existing spec files to match the new convention.",
Copy link
Contributor

Choose a reason for hiding this comment

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

This also seems too broad compared to what we have now. How about:

In this step, we'll automatically rename and/or move your existing spec files as needed.

Which keeps the scope narrow to what we will actually do, and allows it to be correct even if we are only doing one thing or the other. We could leave all other text the same I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually had some variation of this but ended up flip flopping back to "update", but happy to go with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -598,7 +598,7 @@
"description": "Complete the steps below to migrate your project to Cypress 10",
"step1": {
"title": "Rename existing specs",
Copy link
Contributor

Choose a reason for hiding this comment

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

In the screenshot, this title is changed, but it's not here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I might have missed a commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, let's use "Migrate existing specs" for the title. It's the perfect description for what we are doing, it's the migration UI, after all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

I also updated the screenshot.

Copy link
Contributor

@marktnoonan marktnoonan left a comment

Choose a reason for hiding this comment

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

I'm good with this, migrate is indeed a good title imo.

"title": "Rename existing specs",
"description": "In this step, we'll automatically rename and move your existing spec files.",
"title": "Migrate existing specs",
"description": "In this step, we'll automatically rename and/or move your existing spec files as needed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Simpler version, up to you!

Suggested change
"description": "In this step, we'll automatically rename and/or move your existing spec files as needed.",
"description": "In this step, we'll automatically update your existing spec file paths.",

Copy link
Contributor

Choose a reason for hiding this comment

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

Hahaha just caught up on the whole thread on wording between you and @marktnoonan . My 2 cents is that rename and/or move is a bit wordy and makes me think, whereas "update" covers both and I don't have to cerebrally digest what's happening. It's a simple thing we're doing, imo the markup should be short & sweet

Copy link
Contributor

Choose a reason for hiding this comment

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

When I read "update" I feel like like it could do absolutely anything to my files. It's not clear that the changes are limited to moving/renaming, that in this step we aren't going to actually touch the internals of files (like we do other places).

Copy link
Contributor

Choose a reason for hiding this comment

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

What if it's "update your existing spec file paths" instead of "update your existing spec files"?

fwiw I'm not married to either approach and will support whatever you choose 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy with either, just gonna ship this as is. I think they are both fine and combined with the UI, it's pretty clear what's going on.

@lmiller1990 lmiller1990 merged commit e87c492 into develop Jun 16, 2022
@lmiller1990 lmiller1990 deleted the lmiller/21926 branch June 16, 2022 02:14
BeijiYang pushed a commit to BeijiYang/cypress that referenced this pull request Jun 23, 2022
* chore: make spec migration screen text more generic

* update language
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.

Update wording of 'rename specs' area when the specs are not actually being renamed
3 participants