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

Simple and small refactoring - Remove redundant else block in the XPBD simulator #325

Merged

Conversation

kristijanbartol
Copy link
Contributor

Category

  • New feature
  • Bugfix
  • Breaking change
  • Refactoring
  • Documentation
  • Other (please explain)

Description

A very simple pull request, removing the redundant else block, i.e., if and else blocks are identical.

Changelog

  • warp/sim/integrator_xpbd.py -> L2811-2816 (remove the latter three lines).

Before your PR is "Ready for review"

  • Do you agree to the terms under which contributions are accepted as described in Section 9 the Warp License?
  • Have you read the Contributor Guidelines?
  • Have you written any new necessary tests?
  • Have you added or updated any necessary documentation?
  • Have you added any files modified by compiling Warp and building the documentation to this PR (.e.g. stubs.py, functions.rst)?
  • Does your code pass ruff check and ruff format --check?

@kristijanbartol kristijanbartol changed the title Remove redundant else block in the XPBD simulator. Simple and small refactoring - Remove redundant else block in the XPBD simulator Oct 9, 2024
@shi-eric
Copy link
Contributor

shi-eric commented Oct 9, 2024

Hey @kristijanbartol, can you please rebase your changes so that there is only a single commit?

@kristijanbartol kristijanbartol force-pushed the kristijanbartol/tiny-fix-code-redundancy branch from d3bcbe8 to 0717c7c Compare October 10, 2024 09:59
@kristijanbartol
Copy link
Contributor Author

Hi @shi-eric! There should be only a single commit now.

@kristijanbartol
Copy link
Contributor Author

Hi! I might have messed it up by pulling the latest changes to my branch at this point. I don't want to 'git push -f' anything until we agree upon it to avoid further degradation.

Should I now rebase all the commits into one again, with the commit I recently pulled from the main? Sorry for the inconvenience!

@shi-eric
Copy link
Contributor

Hi! I might have messed it up by pulling the latest changes to my branch at this point. I don't want to 'git push -f' anything until we agree upon it to avoid further degradation.

Should I now rebase all the commits into one again, with the commit I recently pulled from the main? Sorry for the inconvenience!

I think it's worth the risk to rebase into a single commit. When I look at the Files changed tab, it's still just the one change you made. Plus, if something unintended happens, you can always close this pull request and make a new one that has only one commit. Sorry to nitpick, just wanted to make sure we don't add more commits than needed!

@kristijanbartol kristijanbartol force-pushed the kristijanbartol/tiny-fix-code-redundancy branch from b37e5ce to bc5a0c2 Compare October 10, 2024 14:56
@kristijanbartol kristijanbartol force-pushed the kristijanbartol/tiny-fix-code-redundancy branch from bc5a0c2 to c6b7563 Compare October 10, 2024 14:57
@kristijanbartol
Copy link
Contributor Author

Great, I think it got sorted out. I force-pushed the squashed commit, and then the PR included additional changes from the latest commit from the main branch. When I rebased (not merged) the main again, I had only my changes visible and the latest main.

It makes sense now, thanks for the support!

@shi-eric shi-eric merged commit f0b0ca4 into NVIDIA:main Oct 10, 2024
1 check passed
@shi-eric
Copy link
Contributor

Thanks for the pull request! Your changes are merged now.

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.

3 participants