search

flomesh-io / pipy

Pipy is a programmable proxy for the cloud, edge and IoT.
https://flomesh.io/pipy
Other
743 stars 70 forks source link

[ci]: do not install deps specified by .spec #138

Closed tchaikov closed 1 year ago

tchaikov commented 1 year ago

We thank you for helping improve Pipy. In order to ease the reviewing process, we invite you to read the guidelines and ask you to consider the following points before submitting a PR:

  1. We prefer to discuss the underlying issue prior to discussing the code. Therefore, we kindly ask you to refer to an existing issue, or an existing discussion in a public space with members of the Core Team. In few cases, we acknowledge that this might not be necessary, for instance when refactoring code or small bug fixes. In this case, the PR must include the same information an issue would have: a clear explanation of the issue, reproducible code, etc.

  2. Focus the PR to the referred issue, and restraint from adding unrelated changes/additions. We do welcome another PR if you fixed another issue.

  3. If your change is big, consider breaking it into several smaller PRs. In general, the smaller the change, the quicker we can review it.

keveinliu commented 1 year ago

Hi Kefu,

Thanks for your update! It's good to put source llvm path in .spec file. But I don't understand why not installing deps specified by .spec? If you don't, the build process will fail. Could you please help explain it to me? Thank you! Kevein

tchaikov commented 1 year ago

Hi Kefu,

Thanks for your update! It's good to put source llvm path in .spec file. But I don't understand why not installing deps specified by .spec?

@keveinliu please refer to the commit message, let me quote it here:

gcc, make and chrpath packages are required by pipy.spec already, and should be installed by "yum-builddep", so there is no need to install them separately.

If you don't, the build process will fail. Could you please help explain it to me? Thank you! Kevein

i don't quite follow your reasoning here. how come the build process will fail?i just double checked, after my change, the build.sh script completed with rpm package generated. as expected, the dependencies including the ones removed in this change were installed in the el7 container.

keveinliu commented 1 year ago

Yes, you're right. I missed yum-builddep just now.