also ignore x86 openssl paths#6416
Conversation
|
Thanks for the quick fix! |
|
Did you try it before submitting? The master is broken now |
|
Proposed fix: #6420 |
|
@Neumann-A you should not abuse CI or use it instead of a proper test on your own computer, also because CI logs are not accessible outside. Using CI in this way (like doing tens of commits in a PR without real necessity) is just abusing moderators' time (they have to report simple errors which would have been easily spotted on ANY machine) and also is using electricity for free with no reasons at all (let's not forget it! Azure-eager for simple PR like this could spawn a LOT of machines and re-test everything [it didn't happen but please do not use this argument and answer to a bug with another bug]! For example I felt so dirty in the past when I was running some hpc simulations on tens of thousands of cpus for tens of hours and then realized my code was bugged...). To be honest, events like this are what could drive people away from It already happened too many times, at least for me, and if it was not for a personal determination to see this project succeed (don't even know why, maybe because I already lost too many hours on it and because I learned a lot in the meantime), I'd have moved away too. I can't stand for a broken master, taken so lightly, merging PR so easily. I really WANT serious code review, not just a green tick which does not mean ANYTHING. I hope you can understand my feelings. Again, not towards anyone, but to explain my position. I want to feel some "responsibility" when I submit a PR, I want to feel some anxiety understanding that I may receive harsh comments because I have been a total jerk in some lines that were totally broken. |
I would say it depends: If it is something which builds quite fast your are correct (around 1-2min). If it is something which requires longer compile times pushing it to CI or a personal build pipeline (if available) is the right thing to do because else you are wasting your personal time and could work on something different.
Nah just putting more pressure on them to finally automate the reporting step ;) Also my work pc has not enough disk space to run every ci build. Furthermore it fails at the end with: so i don't have any informations about regressions which makes the CI command for me useless (unless I want to gather informations from the build logs as done in #5543.)
It happens and there is nothing which could stop it unless you have automatic tests within your codebase (which you should of course have). But sometimes there are new bugs which are not (yet) covered by your tests so you won't notice them until it is too late. So I wouldn't mind it too much because we are all humans and make errors.
Those people should have learnt there lesson long ago and should not directly depend on the master of a public github repository where everything can happen (Living @ head has it risks. maybe vcpkg should have tagged releases or an experimental branch?). People which need vcpkg for work should use a fork or depend on some other clone. That is what I did for our working environment.
Personally, I think the hidden bugs/issues vcpkg currently has are more of a problem then one master breaking commit which has been fixed in less then 12 hours. (e.g. #6368, #5540)
I doubt serious code review will happen since it requires a lot of time. I also think ras0219-msft reviewed the code and decided that it looks ok. If you never encountered the issue before you wouldn't suspect the change of this PR to break anything and that it is why it has been merged so fast. So CR alone also does not tell you anything ;) (But they learned the lesson for #6402). Again we are all humans and make errors.
depending on the level of strictness this could slow down vcpkg progress by a lot. But I would agree on some more strict rules for new ports or better review of new ports in terms of correctness. It would be good if there were a better guide what requirements a new port should fulfill. Then the port can be evaluated against those requirements before a merge to master happens. |
closes #6415