Skip to content
This repository has been archived by the owner on Feb 3, 2021. It is now read-only.

Fix "bad math expression" pmset bug #108

Merged
merged 1 commit into from
Nov 22, 2018
Merged

Fix "bad math expression" pmset bug #108

merged 1 commit into from
Nov 22, 2018

Conversation

matchai
Copy link
Owner

@matchai matchai commented Nov 21, 2018

Description

Ported over the same solution as was made in spaceship.

Motivation and Context

Closes #106

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

I was unfortunately unable to test this fix, though it seems trivial enough not to lead to any issues. We really need to write tests for the battery section.

  • I have tested using MacOS
  • I have tested using Linux

Checklist:

  • I have checked that no other PR duplicates mine
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@Snuggle
Copy link
Collaborator

Snuggle commented Nov 22, 2018

Lookin' good to me.

I wonder why was this grep added in the first place? Tests for the battery section would be amazing, I just don't have a spare moment to add some at the moment. It seems upstream need tests for battery too, but I still need to install npm on my laptop to run their tests, which unfortunately, npm has >200 dependencies on Ubuntu.

@matchai
Copy link
Owner Author

matchai commented Nov 22, 2018

If I'm not mistaken, you should be able to run the spaceship tests without npm by running ./scripts/tests.sh. 🤔

Thanks for the review! 👍

@matchai matchai merged commit 64f06a0 into master Nov 22, 2018
@matchai matchai deleted the bad-math-expression branch November 22, 2018 06:11
@salmanulfarzy
Copy link
Contributor

That's correct @matchai.

@Snuggle There is no need to install npm to run tests of spaceship. npm test is just a convenience wrapper around ./scripts/tests.sh.

You can also run individual tests with ./tests/<name>.test.zsh or zsh tests/<name>.test.zsh

@Snuggle Snuggle added the Type: Bug Fix an existing bug label Nov 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Bug Fix an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad math expression
3 participants