search

arran4 / golang-ical

A ICS / ICal parser and serialiser for Golang.
Apache License 2.0
311 stars 73 forks source link

URL support for parsing (issue #77) #78

Closed Courtcircuits closed 1 month ago

Courtcircuits commented 1 year ago

In response to issue #77, I have made some improvements to the codebase. Specifically, I have added a new method ParseCalendarFromUrl(string) and a new test case that verifies the absence of errors when invoking this method.

Currently, this method initializes a basic http.Client and forwards the response body to the ParseCalendar(string) function. However, as suggested in issue #77, it may be beneficial to allow users to use a custom http.Client to address authentication, CORS, and similar concerns.

I am currently exploring the best design approach to accommodate this feature request. Any input or suggestions on the optimal design for this enhancement would be greatly appreciated.

james-elastic commented 1 year ago

This would be a great help if it was integrated. Thank you!

arran4 commented 1 year ago

Okay noted. I will (try to make time to...) mock the active HTTP get and make what ever changes necessary to do that this weekend. Unless it's something either of you want to do? @james-elastic @Courtcircuits

Courtcircuits commented 1 year ago

By the way, I can enhance this pull request. Currently, there isn't much to it. I'm considering implementing the "functional option pattern" to instantiate the HTTP client (more about this pattern : YouTube video).

@arran4, I can also include a few more tests (e.g mock of HTTP connection). I recognize that the ones I added may not be entirely relevant.

arran4 commented 1 year ago

Thanks.

The functional options pattern is fine, however just a var-args with a type swtich could do as we don't really need meta information around the http.Client.

I am not sure if more tests are warranted as there isn't much logic, definitely need to test the http.Options solution. Also for people have need custom headers, ie authorization, post, etc. You might want to split it into two (or more) functions, where the http.Request is created in the Get and passed to a http.Do function, which itself is also a constructor.

Please try use fmt.Errorf to wrap the errors if it adds value.

arran4 commented 2 months ago

Resolved conflicts in https://github.com/arran4/golang-ical/pull/102

arran4 commented 2 months ago

@Courtcircuits I created a new PR to up date this, can you please review it: https://github.com/arran4/golang-ical/pull/102

I will merge it (or make more changes) in the coming days if no response.

arran4 commented 1 month ago

Yay!