Coverage remained the same at 80.008% when pulling bca650e on shiftstack:port_dates into 2090f9c on gophercloud:master.

I copied what was done in #1671. Should we go back and also add the created_at and updated_at JSON tags for the other networking resources?

@jtopjian Do you happen to remember why the json:"created_at" tags were removed in #1671 while overriding UnmarshalJSON? What am I missing?

One reason might be that when sending the JSON representation of a Port to the OpenStack API, maybe if you send the "wrong" time representation you get an error. For a field that would be ignored anyway. Just guessing.

@pierreprinetti Sorry for the late reply.

This area of code is only for results returned from the API. Requests being sent to the API shouldn't be hitting this code.

The tags were removed so that parsing was picked up by the UnmarshalJSON method. This method is not called if the struct has an explicit JSON tag.

In that method, two different time formats are supported, noted in s1 and s2. If the JSON result contained a time format that wasn't parseable by gophercloud.JSONRFC3339NoZ in s1, then the code attempts to parse the time as time.Time in s2.

However, if s1 was successful, the type is converted to time.Time.

The goal being to support an older style timestamp and a more general/generic timestamp simultaneously.

Let me know if this helps.

@jtopjian
Thank you for taking the time to explain. I however am not yet clear how the tag removal helps.

In a Go playground I have implemented a simplified Port type, and copy-pasted the UnmarshalJSON method. CreatedAt and UpdatedAt have tags. Decoding seems to work: https://go.dev/play/p/FlTrhozjqZr

What am I missing?

hm... So keeping the JSON tags still causes those fields to be processed in the UnmarshalJSON method. This might be a change in behaviour with Golang. There are lots of other places in Gophercloud where a JSON tag of - is used. We would have run into issues years ago if those fields were being omitted from the response body.

Thanks again @jtopjian.
My only concern is about changing the shape of an exported struct between v1.0.0 and v1.y.z. The unit tests run against Go 1.14 and seem to confirm that it's fine to add the tags back. I propose to merge this just to adhere to our promise of compatibility before we ship v1.1.0.

WDYT @mandre?

Thanks again @jtopjian. My only concern is about changing the shape of an exported struct between v1.0.0 and v1.y.z. The unit tests run against Go 1.14 and seem to confirm that it's fine to add the tags back. I propose to merge this just to adhere to our promise of compatibility before we ship v1.1.0.

WDYT @mandre?

No strong feeling either way. Let's merge it then, so that the struct doesn't change.