Skip to content

shorter regex to get the timezone value#5

Open
frederic-mahe wants to merge 2 commits intoMiSTer-devel:masterfrom
frederic-mahe:patch-2
Open

shorter regex to get the timezone value#5
frederic-mahe wants to merge 2 commits intoMiSTer-devel:masterfrom
frederic-mahe:patch-2

Conversation

@frederic-mahe
Copy link
Contributor

@frederic-mahe frederic-mahe commented Jun 4, 2019

first remove everything up to "timezone" included, then remove everything after the actual timezone value.

In my opinion, this is easier to read, and it avoids one subshell spawn by removing a pipe. Saves at least a millisecond :-)

first remove everything up to "timezone" included, then remove everything after the actual timezone value.

In my opinion, this is easier to read, and it saves one subshell spawn by removing a pipe. Saves at least a millisecond :-)
@frederic-mahe
Copy link
Contributor Author

Alternatively, using a more flexible pattern:

TIMEZONE="$(curl -sLf "http://www.ip-api.com/json/" | sed -r 's/.*"timezone" *: *"([^"]*)".*/\1/')"

@vladc
Copy link

vladc commented Dec 15, 2019

Wouldn't this be more appropriate?

TIMEZONE=$(curl -sLf "http://www.ip-api.com/line?fields=timezone");

@frederic-mahe
Copy link
Contributor Author

Thanks @vladc I didn't know it is possible to query json fields directly! @Locutus73 what do you think of that code simplification?

@Locutus73
Copy link
Member

That's brilliant... please excuse me, one of these days during the holydays I'll take some time and I'll review and merge all PR here and in the updater. Thank you.

@sigboe
Copy link

sigboe commented Jun 4, 2020

@frederic-mahe You should update this PR with @vladc suggestion.

To do that locally, check out your patch-2 branch, then apply the change and push it. This PR should get updated automatically.

If you want to get fancy you can (before or after you push the fix) do git rebase -i HEAD~2 this will open up your text editor defined in $EDITOR environment variable, it will open up some lines including some comments, there your two last commits will be, they have the action Pick in front of them. You can take the newest commit and replace the word pick with squash, and save and exit. It will then squash it to the previous commit, the next $EDITOR window opens and you can pick what the commit comment will be.

Then after you squash them you can git push --force to rewrite history :) This PR should be updated, and only have one commit. Everytime you do something github will also write notes about it in this comment section, like in my PR #34

whitespace cleaning, use herestring to avoid a pipe, protect variables
with double-quotes and curly braces.
@frederic-mahe
Copy link
Contributor Author

thanks for the advice @sigboe

I chose to push a second commit, rather than squashing. This is advanced git, but squashing is clearer to me now, thanks to your explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants