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

fix: rename config option name to id #11981

Merged
merged 10 commits into from
Apr 8, 2022
Merged

Conversation

Udit-takkar
Copy link
Contributor

@Udit-takkar Udit-takkar commented Oct 19, 2021

Summary

Fixes #10013
Closes #10051
Closes #10592
Closes #11089
Closes #12235
I have renamed name to id in all the relevant files including tests.
Need help on one test case that is failing in jest/e2e/native-esm/tests/native-esm.test.js

15:1 error node:dnsimport should occur before import ofpath import/order

Test plan

@Udit-takkar
Copy link
Contributor Author

@jeysal i am getting import/order error when running yarn test. But It is already in correct order and after saving that file it moves to it's correct position but still showing error in terminal when i used yarn test

@jeysal jeysal changed the title fix:rename projectConfig.name to projectConfig.id fix: rename config option name to id Oct 19, 2021
@jeysal jeysal force-pushed the rename-name-to-id branch from 1fffca6 to dc19f4d Compare October 19, 2021 13:55
@jeysal
Copy link
Contributor

jeysal commented Oct 19, 2021

@Udit-takkar lint seems to pass on CI - but there are quite a few tests failing, see CI check results for those. Many of them look very easy to fix, once you've got most of them green let me know and any tests that you're not sure how to fix we can look at together

@Udit-takkar
Copy link
Contributor Author

Udit-takkar commented Oct 19, 2021

@jeysal What is the command to test these check like running all tests for /packages/jest-haste-map/src/index.test.js? I have tried few commands like yarn jest but getting error

 n: Status Code is 403 (MongoDB's 404)
This means that the requested version-platform combination doesn't exist
  Used Url: "https://fastdl.mongodb.org/osx/mongodb-osx-ssl-arm64-4.0.14.tgz"

Also I am busy with some hackathon till 27th. Will try resolving after that. Don't mark this one as stale.

@jeysal
Copy link
Contributor

jeysal commented Oct 21, 2021

Looks like an Apple M1 problem (no MongoDB binary available for macos with arm64 architecture). Found this to resolve typegoose/mongodb-memory-server#505

@Udit-takkar
Copy link
Contributor Author

Udit-takkar commented Oct 30, 2021

@jeysal just couple of test are failing now. can you help?

@jeysal
Copy link
Contributor

jeysal commented Nov 1, 2021

Hi - I think that was just a matter of a name in a JS test (which is not type checked) being left over.
PR looks good to me! Of course we will only be able to land this for the next major since it's breaking, so it may need to lay around for a while

@jeysal jeysal added this to the Jest 28 milestone Nov 1, 2021
@Udit-takkar
Copy link
Contributor Author

Ok

@Biki-das
Copy link
Contributor

@Udit-takkar could you please make it a draft as it is a breaking change, this would make the PR page a little bit easy to move around

@Udit-takkar Udit-takkar marked this pull request as draft December 15, 2021 17:44
@Udit-takkar
Copy link
Contributor Author

@Biki-das anything else?

@Biki-das
Copy link
Contributor

Nope😃, thanks!

@mrazauskas
Copy link
Contributor

@SimenB Just to draw your attention, it seems like this PR is ready to land. Marked as a draft, but looking at the conversation becomes clear that the job was finished and it can be merged. If CI is still passing, of course.

Here are few duplicates to be closed after merging: #10051, #10592, #11089, #12235

@SimenB
Copy link
Member

SimenB commented Apr 8, 2022

Thanks @mrazauskas! I'll update it

Copy link
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!

@SimenB SimenB marked this pull request as ready for review April 8, 2022 09:46
@SimenB SimenB merged commit 051af02 into jestjs:main Apr 8, 2022
@github-actions
Copy link

github-actions bot commented May 9, 2022

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 May 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename ProjectConfig.name to ProjectConfig.id
6 participants