Fix #1356

[LittleLittleCloud]

fix #1356

[NiklasGustafsson]

What do the CompatibilitySuppressions files do?

[LittleLittleCloud]

What do the CompatibilitySuppressions files do?

It's like suppress warning.

[yueyinqiu]

It looks like a better approach would be to create the file automatically when packing? since there would still be new stuffs added into TorchSharp in the future, and in the current stage we don't that mind API changes.

[LittleLittleCloud]

@yueyinqiu

Yeah that would be helpful if the suppression file can be generated during packing so we can check in the supression file if necessary as well as detecting the possible unintentional API break change

[yueyinqiu]

Yes but that means we have to separate the pack stage and publish stage? Actually I mean it's a bit early to check for the API compatibility, so we could just generate the suppression file and get into the next step.

[NiklasGustafsson]

@LittleLittleCloud -- is this PR still relevant?

[LittleLittleCloud]

@NiklasGustafsson Yes, I'm thinking of uploading the suppression file to pipeline artifacts so we no longer need to generate it locally. After the suppression file getting uploaded, the workflow for processing API break change will be

• in PR check: the ci fails if the PR includes API break change and upload new suppression file to pipeline artifacts
• if the API break change is on purpose, update the suppression file in PR and fix the validation error
• if the API break change is unexpected, fix the API break change in PR

[NiklasGustafsson]

So we won't have to wait until after merge to find out about the breakage?

[yueyinqiu]

I think a better expression would be we 'shouldn't'. We should know whether a PR will break the API before merging it.

If we only check that before packing and publishing, it's almost impossible to find the specific commit which changed the API and revent it.

[yueyinqiu]

Hmm... are there any news about this? I'm afraid of new commits since they may break APIs which can't even be find out until packing.

By the way, will the fix in #1376 fail due to this as well?

[yueyinqiu]

does #1409 and #1410 solve the problem?

I've also found a way to do this without changing the current api. But that leads to two (almost) independent dataset/dataloader system, which might be confusing.