Skip to content

Conversation

@Aryan1982
Copy link
Contributor

Fixes issue #2150

Changes:
Before:
m1
m2

After:
n1

n2

@welcome
Copy link

welcome bot commented Mar 7, 2023

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already.

@release-com
Copy link

release-com bot commented Mar 7, 2023

Release Environments

This pull request environment is provided by Release, learn more!
To see the status of the environment click on Environment Status below.

🔧Environment Status : https://app.release.com/public/Processing%20Foundation/env-8452e02a7a

lindapaiste
lindapaiste previously approved these changes Mar 7, 2023
@Aryan1982
Copy link
Contributor Author

Annotation 2023-03-10 111748

This check is failing, am I missing something?

@3ru
Copy link
Contributor

3ru commented Mar 10, 2023

@Aryan1982
It is failing on the snapshot test of jest. Please update it below and commit again.

jest --updateSnapshot

@Aryan1982
Copy link
Contributor Author

@3ru
Done.

3ru

This comment was marked as resolved.

@Aryan1982 Aryan1982 requested a review from 3ru March 13, 2023 15:46
@Aryan1982
Copy link
Contributor Author

@3ru
This doesn't feel quiet right.. have I done any mistake?

@Aryan1982
Copy link
Contributor Author

Thank you @3ru , could you tell me what I was doing wrong?

@3ru
Copy link
Contributor

3ru commented Mar 14, 2023

Yes!

What was the problem

The two major problems so far have been

  1. The component was changed, but the corresponding test snapshot was not added.
  2. Library updates (changes to package.json) were included.

Perhaps for some reason, you updated jest and then did a git add . , etc. and you committed everything, but the PR needs to be the minimum change necessary to fix the problem.
For example, in this case, you only need to update the Nav component and the test file for that component, so you only need to commit those two files.

Fixes.

Here are the flow and commands I modified.

  1. Rewind to the very first commit by @Aryan1982

    git reset --hard c544c68c0571dd1acbb99e282e2167710eed1cb5
  2. Force overwrite the rewind to the remote repository

    git push -f origin develop
  3. Update snapshot of just

    jest --updateSnapshot
  4. Add only the snapshot to the commit & push

    git add client/components/__snapshots__/Nav.unit.test.jsx.snap
    
    git commit -m "update nav unit snapshot"
    git push origin develop

It has been a long journey because of my inability to explain.:pray:
Now let's wait for @raclim's review.

Copy link
Contributor

@3ru 3ru left a comment

Choose a reason for hiding this comment

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

lgtm

@Aryan1982
Copy link
Contributor Author

Thank you @3ru for helping me with my first contribution to a open source project. Do you have any socials?

@raclim raclim merged commit a4af1b5 into processing:develop Mar 14, 2023
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.

4 participants