parseDayString

[lukepower]

Added parseDayString function to parse strings like "1d2h30m"

[elcojacobs]

Nice work. Thanks for contributing :)

I think it could be expanded by handling errors a bit better (invalid strings, empty strings). And if no specifier is given (d/h/m), default to days.

And then move it into the class, replacing this function: https://github.com/lukepower/brewpi-www/blob/master/js/profile-table.js#L336

And give it a test, try to break it with the nastiest input you can come up with.

[lukepower]

Hm actually I have a more curreent version which has not been updated correctly.. Have to check that.

[elcojacobs]

The string parsing seems to work nicely. I only just found a new issue: we sorted the tables based on the day column. Having these strings in there breaks the sort. We need to sort by date instead. After testing, the console.log statements should probably be removed.

Now for the pedantic, but educational part: Pull requests are easiest to merge if they are against the up-to-date develop branch. This involves 3 steps:

• Create a new branch from your latest commit, called feature/timestrings. You would normally create the branch before you start working.
• Reset the master branch back to the master on the remote, never work on the master branch.
• Change this pull request to merge your feature branch into develop

Ideally, also this step:

• Rebase (and maybe squash) your changes onto the latest develop branch. But this is quite advanced git stuff. If you add me on Skype, I can help you with that. https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request

[lukepower]

Hi Elco,

I see… I definitively need to understand the Git way of updating better, as I only used it till now to get code :)

You can catch me on Skype (lukas_demetz) so maybe we could do a sample procedure together.

As for the sorting by date, I will have alook at it. Either I can provide the sorting with the calculated day value, or I could add a hidden column with the day values and sort by that.

Cheers

Lukas

Von: Elco Jacobs [mailto:notifications@github.com] Gesendet: Donnerstag, 3. September 2015 00:38 An: BrewPi/brewpi-www brewpi-www@noreply.github.com Cc: lukepower lukas@lukepower.com Betreff: Re: [brewpi-www] parseDayString (#50)

The string parsing seems to work nicely. I only just found a new issue: we sorted the tables based on the day column. Having these strings in there breaks the sort. We need to sort by date instead. After testing, the console.log statements should probably be removed.

Now for the pedantic, but educational part: Pull requests are easiest to merge if they are against the up-to-date develop branch. This involves 3 steps:

• Create a new branch from your latest commit, called feature/timestrings. You would normally create the branch before you start working.
• Reset the master branch back to the master on the remote, never work on the master branch.
• Change this pull request to merge your feature branch into develop

Ideally, also this step:

• Rebase (and maybe squash) your changes onto the latest develop branch. But this is quite advanced git stuff. If you add me on Skype, I can help you with that. https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request

— Reply to this email directly or view it on GitHub https://github.com/BrewPi/brewpi-www/pull/50#issuecomment-137264645 . https://github.com/notifications/beacon/AEdwr8gSIZgB-kPhYWkKe4mSBmcWhaDcks5ot3HigaJpZM4F0AjP.gif

[lukepower]

Ok Elco, I fixed the sorting issue, should now work fine.

[elcojacobs]

Closed, merges as a squashed commit in a different pull request