search

kubevirt / community

Community content
https://kubevirt.io
49 stars 103 forks source link

Proposal: New coding standards for properly use contexts in functional tests #304

Open nunnatsa opened 3 months ago

nunnatsa commented 3 months ago

Release note:

New coding standards for properly use contexts in functional tests
kubevirt-bot commented 3 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign cwilkers for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubevirt/community/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
nunnatsa commented 3 months ago

/cc @0xFelix /cc @fossedihelm /cc @jean-edouard /cc @EdDev

fabiand commented 3 months ago

:wave:

How does this relate to the work that Felix did in kubevirt/kubevirt repo. Why do we have some parts here and some parts there? How will we ensure that developers know where to look?

nunnatsa commented 3 months ago

👋

How does this relate to the work that Felix did in kubevirt/kubevirt repo. Why do we have some parts here and some parts there? How will we ensure that developers know where to look?

Not sure I'm familiar with the work Felix did. The suggestion is to add the coding standards to the kubevirt/kubevirt repo.

My PR was merged but is about to be reverted, because there was no proper discussion about it: https://github.com/kubevirt/kubevirt/pull/12148

EdDev commented 3 days ago

This makes so much sense to me, not quite sure why it's so controversial…

It is controversial because in the current form of the tests, it will make them harder to read and add a lot of boilerplate code. AFAIK it mainly got stuck due to priority. There are other issues to handle in our codebase which have precedence.

We will also need all SIGs approval to enforce this. I’m unsure if all will be able to commit following this.