Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add other Components creation and properties methods (#65) #66

Closed
wants to merge 0 commits into from

Conversation

spslater
Copy link
Contributor

@spslater spslater commented Apr 3, 2023

Changes for issue #65:

  • Add methods to create, add, and get Todo, Journal, Timezone, and Free/Busy components to a calendar and moved the Alarm methods to live under the VAlarm struct.
  • Todo and Journal have setters for the properties listed in RFC5545 and RFC7968
  • Moved a bunch of the methods attached to VEvent to ComponentBase so that the Todo and Journal objects would also have access to them, without needing to duplicate the methods
  • For a couple of the methods like setPriority and setResources I made them private on the ComponentBase and then added public methods on the Components they're valid in, Event and Todo. I'm not super married to this idea, there is probably a better way to handle this without duplicating too much code.
  • I added new things to the bottom of the constant lists, and left all of the ComponentBase (previously Event) methods in the same order.
  • The different components are ordered:
    • Struct
    • serialize methods (serialize, Serialize)
    • New method
    • Two Add methods (AddEvent, AddVEvent)
    • Getting all components method (Events)
    • Any property setters only for that component (SetEndAt)
    • Property setters that are shared between some (but not all) components (SetPriority)
    • Any existing getters (GetEndAt)

@spslater
Copy link
Contributor Author

spslater commented Apr 3, 2023

I've already notice a typo in components.go where the VTodo setGeo method should be SetGeo to make it public. I will hold off on pushing another commit in case any changes need to be made.

@spslater spslater changed the title Add Todo and Journal creation and properties and Timezone, Free/Busy, and Alarm creation Add other Components creation and properties methods (#65) Apr 3, 2023
@spslater
Copy link
Contributor Author

spslater commented Apr 4, 2023

Workflow failed seems to be caused by an incident at Github, issue is resolved now, so we should be able to just rerun it.

https://www.githubstatus.com/incidents/0d1bcqt29hm2

@arran4
Copy link
Owner

arran4 commented Apr 6, 2023

Okay attempting to re-run them all

@arran4
Copy link
Owner

arran4 commented Apr 6, 2023

@spslater It still seems to be failing. However for much clearer reasons:

Run go test ./...
go: downloading github.com/stretchr/testify v1.7.0
go: downloading github.com/pmezard/go-difflib v1.0.0
go: downloading gopkg.in/yaml.v3 v3.0.0-20210107192922-49[6](https://github.com/arran4/golang-ical/actions/runs/4600085447/jobs/8187337848?pr=66#step:5:7)545a630[7](https://github.com/arran4/golang-ical/actions/runs/4600085447/jobs/8187337848?pr=66#step:5:8)b
go: downloading github.com/davecgh/go-spew v1.1.1
# github.com/arran4/golang-ical [github.com/arran4/golang-ical.test]
Error: ./components.go:6[9](https://github.com/arran4/golang-ical/actions/runs/4600085447/jobs/8187337848?pr=66#step:5:10)9:4: syntax error: unexpected newline, expecting comma or }
FAIL	github.com/arran4/golang-ical [build failed]
FAIL
Error: Process completed with exit code 2.

@spslater
Copy link
Contributor Author

spslater commented Apr 6, 2023

Whoops, not sure how I missed that, fixed it and a couple other things I caught.

@arran4
Copy link
Owner

arran4 commented Dec 26, 2023

@spslater There is a conflict.

@arran4 arran4 linked an issue Dec 26, 2023 that may be closed by this pull request
@arran4
Copy link
Owner

arran4 commented Jan 8, 2024

@spslater I have attempted to resolve the conflict. Please take a look at some stage.

@arran4
Copy link
Owner

arran4 commented Jan 8, 2024

Bah I broke it.

@arran4
Copy link
Owner

arran4 commented Jan 8, 2024

Okay.. Github merge helped me mangle that.

@arran4
Copy link
Owner

arran4 commented Jan 8, 2024

Force pushed removal. Shouldn't have closed it though.

@spslater
Copy link
Contributor Author

spslater commented Jan 8, 2024

Looks like that force push overwrote some of the changes, I will see if I can get a clean merge with some small changes from my local copy.

@arran4 arran4 mentioned this pull request Jan 8, 2024
@arran4
Copy link
Owner

arran4 commented Jan 8, 2024

Bah trashed more commits than I should have.

I had to restart the merge please look at #84

@arran4
Copy link
Owner

arran4 commented Jan 8, 2024

Looks like that force push overwrote some of the changes, I will see if I can get a clean merge with some small changes from my local copy.

I think I salvaged it. Perhaps compare?

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

Successfully merging this pull request may close these issues.

VTodo and VJournal Missing Setters and Getters
2 participants