search

eclipse-cyclonedds / cyclonedds-python

Other
54 stars 44 forks source link

Solving IDL generator with structs inheritance which identifiers are Python keywords #174

Open Bit-Seq opened 1 year ago

Bit-Seq commented 1 year ago

Solved the error detailed on #173.

thijsmie commented 1 year ago

Thanks @Bit-Seq! I think this change is okay, we are just having some CI troubles right now. I'll rerun when that is resolved and come back to this PR.

Bit-Seq commented 1 year ago

Struct inheritance tests are in https://github.com/javiersorianoruiz/cyclonedds-python-checkReservedKeyword/tree/master/tests/. All the 007_test_X are related to it.

Bit-Seq commented 1 year ago

Hello!

Have you been able to solve CI troubles?

We (@jordialexalar and me) have created tests related to reserved Python keywords support using pytest. For a mistake we have added this file in the same fork as the pull requests pending to merge. All pull requests are connected, and also with the issues #105, #162, #163, #172 and #173.

Sorry for the inconvenience. If there is any problem we can create another fork and pull request with the tests.

Regards.

thijsmie commented 1 year ago

Hi @Bit-Seq,

Yeah I did but the PR has not been reviewed yet (see #176). As soon as that is merged rerunning this should also succeed.

eboasson commented 1 year ago

I was discussing the status of this PR with @NoxPardalis and triggered a custom run of the CI for this PR to confirm that all is well with #176 merged, but it turns out there is some problem: https://dev.azure.com/eclipse-cyclonedds/cyclonedds-python/_build/results?buildId=3867&view=results . The results of this run are not visible on GitHub because, well, reasons, but if I am not mistaken I started it the right way.

I haven't looked at all the failures, but my sample suggests it consistently fails in the test_datatypes_keywords tests. I haven't dug into it (too many things to do, too little time ...), but would you perchance have some time to look into it, @Bit-Seq ?

On a side note: if the test run were successful, merging it would still be blocked anyway on the Eclipse Contributor Agreement verification.

eboasson commented 1 year ago

Cat on keyboard 😂