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.

Sign up for GitHub

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

Jump to bottom

feat: Add vue2 package from npm/vue/v2 branch #21026

Merged
merged 20 commits into from
Apr 13, 2022
Merged

feat: Add vue2 package from npm/vue/v2 branch #21026

merged 20 commits into from
Apr 13, 2022

Conversation

BlueWinds
Copy link
Contributor

@BlueWinds BlueWinds commented Apr 11, 2022 •
edited
Loading

User facing changelog

  • bundles npm/vue2 in the binary, accessible via cypress/vue2.

Additional details

Part of the component testing rearchitecture, to allow us to easily bring updates to users w/o requiring them to install anything other than cypress.

How has the user experience changed?

PR Tasks

  • Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • Has a PR for user-facing changes been opened in cypress-documentation?
  • [n/a] Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Apr 11, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Apr 11, 2022 •
edited
Loading


Test summary

4336 • 0 • 48 • 0 • Flakiness 0

Run details
Project cypress
Status Passed
Commit 1f9f619
Started Apr 13, 2022 3:59 PM
Ended Apr 13, 2022 4:13 PM
Duration 14:09 💡
OS Linux Debian - 10.10
Browser Electron 94

View run in Cypress Dashboard ➡️

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

tgriesser
tgriesser requested changes Apr 11, 2022
View reviewed changes
Copy link
Member

@tgriesser tgriesser left a comment

Choose a reason for hiding this comment

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

2 things - I thought we had landed on vue-2 for the package name

Also, we should skip copying over all of the Cypress tests & examples dir from the existing Vue project

@BlueWinds
Copy link
Contributor Author

2 things - I thought we had landed on vue-2 for the package name

Also, we should skip copying over all of the Cypress tests & examples dir from the existing Vue project

The only discussion I saw was in the Jira ticket, where Lachlan commented:

Can we name it vue2 instead of vue-2? I think it looks way nicer. Also there is some existing literature, like jest27-vue2, etc.

and it seemed reasonable to me. If there was any further discussion, I was unaware, [email protected] looks quite nice to me, vs [email protected]. The dash feels weird in contexts where we don't see only the module name. But I'm not married to the idea if other people have strong preferences / there was discussion I missed.

@tgriesser
Copy link
Member

and it seemed reasonable to me. If there was any further discussion, I was unaware, [email protected] looks quite nice to me, vs [email protected]. The dash feels weird in contexts where we don't see only the module name. But I'm not married to the idea if other people have strong preferences / there was discussion I missed.

That's fine - I was just going based on the Jira ticket title/body, didn't see that comment. @brian-mann or @ZachJW34 do you have a strong opinion on naming here

@brian-mann
Copy link
Member

brian-mann commented Apr 12, 2022 •
edited
Loading

vue2 is fine i don't have a strong preference. I think the original reason why I suggested the dash is to make it clearer that it's about vue AT THE VERSION of 2 instead of feeling like it's the "newer vue".

vue2 vs vue-2 where the -2 is a slightly clearer signal that its vue... but version 2. There's fairly common conventions in the node community when there's a fork of a original project that they'll append the 2 to the namespace to indicate that this is "a newer version of the original" and that's what I was trying to avoid. But really I don't care, I agree that vue2 looks nicer, and as long as people think that vue > vue2 we're good. If we think the opposite could be interpreted, then i'd stick with vue-2

@lmiller1990
Copy link
Contributor

lmiller1990 commented Apr 13, 2022 •
edited
Loading

Regarding names, I see @brian-mann's point now - in the project I maintain, vue-jest, we actually have vue2-jest27 and vue3-jest27, so there's no ambiguity. We aren't doing that here, though, so I'm not sure if vue2 or vue-2 is better. I still think vue2 looks nicer, shall we just go with that?

@lmiller1990 lmiller1990 marked this pull request as ready for review April 13, 2022 07:15
@lmiller1990 lmiller1990 requested review from a team as code owners April 13, 2022 07:15
@lmiller1990 lmiller1990 requested review from jennifer-shehane, lmiller1990 and a team and removed request for a team April 13, 2022 07:15
@lmiller1990 lmiller1990 requested review from ZachJW34 and removed request for jennifer-shehane and a team April 13, 2022 07:18
@tbiethman tbiethman requested a review from ryanthemanuel April 13, 2022 15:44
@tgriesser tgriesser merged commit 3aa69e2 into 10.0-release Apr 13, 2022
@tgriesser tgriesser deleted the UNIFY-1253-npm-vue2 branch April 13, 2022 16:17
@BlueWinds BlueWinds mentioned this pull request Apr 13, 2022
Merged
1 task
tgriesser added a commit that referenced this pull request Apr 13, 2022
@tgriesser
Merge branch 'tgriesser/fix/UNIFY-1206-cloud' of github.com:cypress-i…
fdac9d8 
…o/cypress into tgriesser/fix/UNIFY-1206-cloud

* 'tgriesser/fix/UNIFY-1206-cloud' of github.com:cypress-io/cypress:
  feat: swap the #__cy_root id selector to become data-cy-root for component mounting (#20951)
  fix: Doc changes around vue2 (#21066)
  feat: Add vue2 package from npm/vue/v2 branch (#21026)
  fix: add possible frameworks to object API config (#21056)
tgriesser added a commit that referenced this pull request Apr 14, 2022
@tgriesser
Merge branch '10.0-release' into lmiller1990/UNIFY-1402
c190faf 
* 10.0-release: (25 commits)
  fix: stop running spec when switching specs (#21038)
  fix: remove asset size warnings and enable nuxt e2e tests (#21074)
  feat: swap the #__cy_root id selector to become data-cy-root for component mounting (#20951)
  fix: Doc changes around vue2 (#21066)
  feat: Add vue2 package from npm/vue/v2 branch (#21026)
  skip failing test
  fix: add possible frameworks to object API config (#21056)
  fix snapshot spacing
  fix system test
  fix snapshot
  update snapshots
  rename spec files
  rename files
  update snapshots
  release 9.5.4 [skip ci]
  fix(regression): cy.pause() should not be ignored with `cypress run --headed --no-exit` (#20877)
  fix: add missing Cypress.Commands.addAll() types (#20894)
  chore: Don't store video and screenshot artifacts for runs (#20979)
  chore: Update Chrome (beta) to 101.0.4951.26 (#20957)
  chore: remove parallelism from test-binary-against-repo jobs (#21004)
  ...
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 1, 2022

Released in 10.0.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v10.0.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lmiller1990 lmiller1990 lmiller1990 approved these changes

@tgriesser tgriesser tgriesser approved these changes

@ZachJW34 ZachJW34 Awaiting requested review from ZachJW34

@ryanthemanuel ryanthemanuel Awaiting requested review from ryanthemanuel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BlueWinds @tgriesser @brian-mann @lmiller1990