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

Upgraded to use both UTC offset timezones and named timezones. #151

Merged
merged 5 commits into from
Jul 23, 2015

Conversation

DiegoZoracKy
Copy link

applyTimezone upgraded to accept UTC offset timezone, using the .zone setter from moment.

@urish
Copy link
Owner

urish commented Jul 23, 2015

Thanks!

Looks good in general, though, it seems like the method of distinguishing between timezone names and offsets would miss some timezone names that do not include a slash character, such as "EST". Can you please fix that and update the PR?

thx!

…amed timezone. Plus, using the new .utcOffset instead of .zone setter.
@DiegoZoracKy
Copy link
Author

You are correct. I had to do this in a hurry (working on a project) and i haven't thought so well about it before the pull. I created a new version of the code but it seems that it didn't went well with travis. I will try to take a look later.

@urish
Copy link
Owner

urish commented Jul 23, 2015

thanks man!

@DiegoZoracKy
Copy link
Author

"There was an error while loading data, please try again" is the error shown by Travis. Maybe you can re-run the test?

@urish
Copy link
Owner

urish commented Jul 23, 2015

I just re-ran it, seems to be failing:

https://travis-ci.org/urish/angular-moment/builds/72359159

Uri

@DiegoZoracKy
Copy link
Author

Check

@urish
Copy link
Owner

urish commented Jul 23, 2015

looks great!

One last thing - can you please add a test case for the UTC offset timezone feature?

thx!

@DiegoZoracKy
Copy link
Author

Done!

urish added a commit that referenced this pull request Jul 23, 2015
Upgraded to use both UTC offset timezones and named timezones.
@urish urish merged commit 27054ae into urish:master Jul 23, 2015
@urish
Copy link
Owner

urish commented Jul 23, 2015

You are awesome, million thanks!

@DiegoZoracKy
Copy link
Author

Nice! Soon i will have to make another PR for utilizing timezone at amCalendar

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants