Add method to remove property

[quite]

Fixes https://github.com/arran4/golang-ical/issues/95

[arran4]

Thanks. I have reviewed. The fix might solve the lint issue too.

[quite]

lint

The lint issue is probably caused by the github action is not locked to a specific version of golangci-lint. So whenever things change in new version, the next run of the ci might trigger some lint errors that were not errors before. So we get unrelated lint complaints on PRs, like here.

I'd suggest locking golangci-lint to a specific version, and fixing the lints on main. Then yes, bumping the linter will have to become a regular chore, but at least PRs won't be hit by random new lint rules.

[quite]

Added test though

[arran4]

@quite I've merged a PR that conflicts with components_test.go would be nice of github to provide warnings for that IMHO. If you resync it should help with lint resolution depending on what's going on

[quite]

rebased, i think lint issues should be done

[arran4]

When do you need this released? Soon, or you can wait for the #73 ?

[quite]

You mean it will return a nil slice if the last prop is removed right? I think that is fine, since in practice there is no difference between a nil slice and an empty slice. Go styleguide also: https://google.github.io/styleguide/go/decisions#nil-slices

On 28 September 2024 09:03:19 CEST, Arran Ubels @.***> wrote:

@arran4 approved this pull request.

Will have to reapprove once the lint issue is resolved. Comments applied

@@ -89,6 +89,18 @@ func (cb *ComponentBase) AddProperty(property ComponentProperty, value string, p cb.Properties = append(cb.Properties, r) }

+// RemoveProperty removes from the component all properties that has +// the name passed in removeProp. +func (cb *ComponentBase) RemoveProperty(removeProp ComponentProperty) {

• var keptProperties []IANAProperty
• for i := range cb.Properties {
• if cb.Properties[i].IANAToken != string(removeProp) {
• keptProperties = append(keptProperties, cb.Properties[i])

It isn't a core concern of the library, as I found another area where I preference memory over performance.

I guess the real issue here is if people are using the slice directly. For simplicity I guess it's better to go with yours for now.

However there are lint issues remaining if you resync it should help.

One thing your version will do is put a nil slice instead of an empty slice. Which is different, but I don't know if it's a concern:

https://go.dev/play/p/iRgj5Jsqe8O

[quite]

Sure, no hurry

On 28 September 2024 09:15:22 CEST, Arran Ubels @.***> wrote:

When do you need this released? Soon, or you can wait for the #73 ?

[arran4]

You mean it will return a nil slice if the last prop is removed right? I think that is fine, since in practice there is no difference between a nil slice and an empty slice. Go styleguide also: https://google.github.io/styleguide/go/decisions#nil-slices … On 28 September 2024 09:03:19 CEST, Arran Ubels @.**> wrote: @arran4 approved this pull request. Will have to reapprove once the lint issue is resolved. Comments applied > @@ -89,6 +89,18 @@ func (cb ComponentBase) AddProperty(property ComponentProperty, value string, p cb.Properties = append(cb.Properties, r) } +// RemoveProperty removes from the component all properties that has +// the name passed in removeProp. +func (cb *ComponentBase) RemoveProperty(removeProp ComponentProperty) { + var keptProperties []IANAProperty + for i := range cb.Properties { + if cb.Properties[i].IANAToken != string(removeProp) { + keptProperties = append(keptProperties, cb.Properties[i]) It isn't a core concern of the library, as I found another area where I preference memory over performance. I guess the real issue here is if people are using the slice directly. For simplicity I guess it's better to go with yours for now. However there are lint issues remaining if you resync it should help. One thing your version will do is put a nil slice instead of an empty slice. Which is different, but I don't know if it's a concern: https://go.dev/play/p/iRgj5Jsqe8O

Yeah I had to verify that. It's maps which you aren't so lucky, kinda wished they were both one way or the other.