Exclude development scripts from published package#60
Exclude development scripts from published package#60Manishearth merged 1 commit intoyaahc:masterfrom
Conversation
Manishearth
left a comment
There was a problem hiding this comment.
Please, please, stop making these PRs without including tests/examples/etc.
Perhaps just use explicit excludes instead? Makes it clearer to the reviewers what the problem files are, instead of foisting the burden on the reviewer to do this.
I would again highlight that this is personal preference whether to include or exclude examples. I accept that you want them to be there, but I just don't know in which crates your are currently involved and in which not.
I'm happy to provide a list of these files if necessary: For displaydoc it's the |
During a dependency review we noticed that the displaydoc crate includes various development scripts. These development scripts shouldn't be there as they might, at some point become problematic. As of now they prevent any downstream user from enabling the `[bans.build.interpreted]` option of cargo deny. I opted for using an explicit include list instead of an exclude list to prevent these files from being included in the published packages to make sure that everything that's included is an conscious choice.
5ad8efd to
3acb588
Compare
|
Adjusted the PR to include tests as requested (no examples or benchmarks exist for this crate) |
Yes, I'm well aware that there are conflicting pressures to include/exclude tests: as a maintainer I get asked to go both ways on the semi-regular. But you don't need to think about that in these PRs: that personal preference is likely to be reflected in the current value of the include/exclude key, which you can check when making these PRs. Whatever the reviewer preference may actually be, it would not be controversial to keep the status quo, and if these PRs is how reviewers discover they have a preference counter to the status quo, that's great, that's their problem. I think it's misleading to make PRs that say that they exclude development scripts when in practice they lead to a lot of other things being excluded. It's harder on the reviewer since the reviewer actually has to go check, and not all reviewers will remember to do this. It's not even clear exactly what files are problematic from these PRs, e.g. I'm super wary about rust-fuzz/libfuzzer#137 because it's a native build and I don't want to break it, but also I don't actually know what the problem files are. |
During a dependency review we noticed that the displaydoc crate includes various development scripts. These development scripts shouldn't be there as they might, at some point become problematic. As of now they prevent any downstream user from enabling the
[bans.build.interpreted]option of cargo deny.I opted for using an explicit include list instead of an exclude list to prevent these files from being included in the published packages to make sure that everything that's included is an conscious choice.