-
Notifications
You must be signed in to change notification settings - Fork 248
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
Export and refactor har.Logger.Entries #311
base: master
Are you sure you want to change the base?
Conversation
har/entrylist.go
Outdated
func (el *EntryList) Lock() { | ||
el.lock.Lock() | ||
} | ||
|
||
func (el *EntryList) Unlock() { | ||
el.lock.Unlock() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to expose the lock/unlock out of the EntryList struct? The functions on EntryList can work with el.lock directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the thought was that if someone wanted to create a particular variant of a har file - for example, someone wanted all hars that had a URL of "*.google.com" - they would need to be able to iterate the list. Without the lock being exposed that would end up non goroutine safe.
However, my problem may very well be that my brain is still stuck at times in "ruby mode" where one can sort of stick their toe in and out of an object/struct as they please. I'm thinking the better option to handle my thought process would be for a container interface that one could implement in any way they wished outside of martian. That would allow for the locks to be unexported because the internal implementation would be solely for the purposes of martian itself and if one wanted the "*.google.com" concept they would implement a new container interface and set the logger to use that.
In fact nothing would need to be exported from martian but rather if martian allowed the container interface implementation to be set then the caller would have complete control of the container outside of martian.
Would your preference be to close this pull or to rework within this request?
Thanks for your patience on this......
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to just add another commit to this pull request because the difference was not that much upon adding an interface.
32c92a5
to
becf132
Compare
I would like to be able to pull out a single har entry from the har.Logger without it being exported to json first. In order to accomplish this - I've exported the har.Logger.Entries field and I refactored the field to utilize a standard container/list as opposed to a custom double linked list. I believe this will help utilizes the Entries field better without the need to know what the underlying container is.
For example, if one wanted to add a function to export a single har file then it is easier to call RemoveEntry than it is to code against the customize double linked list.
Tests are included for the new sruct