-
Notifications
You must be signed in to change notification settings - Fork 1
bump up xla version #93
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM~
@i-chaochen fixed! |
Same as other PR. Linking to tickets or explain why this change is needed takes like two minutes of time and will potentially save hours months from now. |
@charleshofer it is xla team's request to update the pinned commit to the latest. |
Okay, but why are they requesting this? What does this change or fix and why are those changes necessary? Imagine yourself 3 months from now looking back at this PR. What type of information would you want to know right away if you suspect that this PR introduced a bug or a specific feature? The person or team who requested it probably isn't that important, but why they thought it was important is. That's the type of info that's helpful in commit comments. |
I realize this it's nit-picky, but in my experience, consistently having good comments makes future development easier. Here's an example of a pretty good one: #50. It's super clear from the comment that the change adds a patch file that changes the installed jaxlib, that we need this change to fix hipsolver unit tests when using the plugin, and it gets bonus points for reminding future developers of what they need to do if they move the file that the PR adds. |
I agree @charleshofer about the commit message, bump up xla according to chao's request this is sloppy...don't put my name on it and don't edit my message, please. As a side note, I notice that seems this |
@Ruturaj4 is this PR outdated? If so, please close the PR. |
@i-chaochen bump xla on master.