[github] Use Option<&str> instead of Option<DateTime> for `list_workf…

[chantra]

…low_runs'screated` parameter.

The issue is described in more details in https://github.com/github/rest-api-description/issues/2088

For now, this change will treat the created parameter as an optional str, which it would probably do if the parameter had a custom format defined. Alternatively, one oculd try to implement a proper datetime rnage that follows the Query for dates format, but this is maybe more work than one would benefit by implementing the search formating on the user side.

[chantra]

Notes:

• the relevant changes are really only in the generator. The rest came by just rebuilding the project. I am happy to strip those changes out.
• I only rebuilt the github project, and disabled sync-ing the spec from github as it would fail to build with a bunch of errors not directly related to this.

[chantra]

@augustuswm this is also another PR I am looking for feedback. I am hoping the upstream API spec will get fixed, but in any cases, it seems this type of field will require special casing.

[augustuswm]

I think the idea behind the change makes sense, the created component certainly should not have a format property.

The change though needs to be more specific. As-is it is changing the generator so that anytime a parameter named created is encountered it alters the output type. What this needs to do is to alter the specific component, and only in the case that we are generating a GitHub client.

I will try to find some time to look at how to get this integrated

[chantra]

The change though needs to be more specific. As-is it is changing the generator so that anytime a parameter named created is encountered it alters the output type. What this needs to do is to alter the specific component, and only in the case that we are generating a GitHub client.

yeah, agreed. I did not find a way to compare to the "parent" spec.

[chantra]

@augustuswm I updated this diff. Here is the current change:

• I get proper_name to trickle down to ParameterDataExt::render_type in order to be able to only change the handling of the data type when within github context
• removed the generated files
• I rebased on top of main

I am not sure there is much better options to handle ParameterDataExt::render_type better than passing proper_name as context.

[chantra]

@augustuswm not sure if you got as chance to look at this update.

[chantra]

@augustuswm any opinion or objections on the latest proposal? Thanks

[augustuswm]

I'll look at this today. Main thing I need to ensure is that this change only affects the necessary operations. (i.e. it doesn't leak to other parameters named created if there are any)

[chantra]

So currently it will only change for GitHub project… but possibly any created parameter of that spec. IIRC, there is a couple of references to created and they all have the same fault, but it is not impossible that a legit datetime appears in this spec in the future.

[chantra]

Another possibility would be to patch the spec. This way, there is no need to mess around within the code to special case stuff.

This is probably the simplest and decently easy? WDYT?

[augustuswm]

I went the path of patching the spec file (and it is now updated on main). If that looks correct I'll close this.

[chantra]

Thanks! That look correct. I will try to rebase and run program on top of main to confirm this is actually doing what is expected.

The only follow-up I would have is to keep the patch in the source tree and try to apply it on spec update, just to make sure that the change does not get lost over time.

[chantra]

I went the path of patching the spec file (and it is now updated on main). If that looks correct I'll close this.

ok, I tried my program on top of main and it works as expected.

I have #66 as a follow-up to make sure that the patch get applied on subsequent spec update until github/rest-api-description#2088 get resolved.

[chantra]

@augustuswm I am going to close this PR given that the spec change works.

I think we should have something along #66 though to make sure this does not regress. WDYT?