Skip to content

Conversation

@mkopec
Copy link
Member

@mkopec mkopec commented Apr 10, 2025

This reverts commit 63b3ec7.

Change-Id: I2ecf24f7066e60dc1df89d0a500c913f762f4862

@mkopec mkopec requested a review from miczyg1 April 10, 2025 10:32
@mkopec mkopec self-assigned this Apr 10, 2025
@miczyg1
Copy link
Contributor

miczyg1 commented Apr 10, 2025

I won't approve it without @pietrushnic consent

@miczyg1 miczyg1 requested a review from pietrushnic April 10, 2025 12:13
@pietrushnic
Copy link
Contributor

pietrushnic commented Apr 10, 2025

@miczyg1 thanks for giving me chance to be a gatekeeper.

@mkopec I'm on board with the proposed plan, but I want to make sure we document the versioning guides for Dasharo properly. Consider this an opportunity for learning and growth, so there must be some form of accountability for the oversight 😜 . Here's what needs to be included:

  • Historical Foundation: The Dasharo downstream coreboot code bases that served as the foundation for prior releases can be found at coreboot-1 and coreboot-2. For edk2, reference edk2-1 and edk2-2. Each of these has supported at least two releases on different laptops, targeting diverse user bases, over a span of no fewer than five months. 📅

  • Sales Validation: Our internal metrics confirm that a significant volume of hardware devices were sold over the stated period. This sales volume acts as one of three key indicators—along with the above-stated historical foundation—to justify elevating this codebase to production-level versioning. 📦

  • Testing Metrics: With 805 executed tests and an impressive pass rate of 98.75% (790 out of 805), we not only meet but exceed the arbitrary thresholds set for production versioning: a minimum of 800 tests executed and a pass rate of 98%. This robust performance justifies its promotion to v1.x.y. ✅

@wessel-novacustom cc

TL;DR I will approve when I will merge guideline for that first in our versioning documentation.

@mkopec
Copy link
Member Author

mkopec commented Apr 10, 2025

Added prod version guideline in Dasharo/docs#1044

pietrushnic
pietrushnic previously approved these changes Apr 10, 2025
@pietrushnic
Copy link
Contributor

@mkopec thanks, merged and approved this PR.

Copy link
Contributor

@miczyg1 miczyg1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing Metrics: With 805 executed tests and an impressive pass rate of 98.75% (790 out of 805), we not only meet but exceed the arbitrary thresholds set for production versioning: a minimum of 800 tests executed and a pass rate of 98%.

Now that we have clear rules, I may throw in my 2 cents.

I don't see 800 tests for the latest release: https://github.com/Dasharo/osfv-results/blob/main/boards/NovaCustom/MTL_14th_Gen/V560TNX/v0.9.1-results.csv (barely 624, where there are a lot of skips, so basically test not executed because of unmet conditions or different reasons, there are also rows with TBD, so basically no tests at all). Where is this data coming from?

If we are considering 800 all together for all variants, that becomes relatively easy to overcome by introducing more variants. MSI has at least 2 variants (DDR4 and DDR5). If we count WIFI and no-WIFI, we have 4 variants. One variant needs barely 200 tests (including skips and TBDs?). That's... at least not serious.

Unless I don't udnerstand this requirement and we mean all executed tests for releases. Do RC tests also count? Then we could probably qualify very quickly...

@pietrushnic @mkopec

@pietrushnic
Copy link
Contributor

I don't see 800 tests for the latest release: https://github.com/Dasharo/osfv-results/blob/main/boards/NovaCustom/MTL_14th_Gen/V560TNX/v0.9.1-results.csv (barely 624, where there are a lot of skips, so basically test not executed because of unmet conditions or different reasons, there are also rows with TBD, so basically no tests at all). Where is this data coming from?

I'm counting on both releases, not one:

user@work:~/Downloads$ grep FAIL v0.9.1-results\ \(1\).csv |wc -l
8
user@work:~/Downloads$ grep PASS v0.9.1-results\ \(1\).csv |wc -l
388
user@work:~/Downloads$ grep PASS v0.9.1-results.csv |wc -l
402
user@work:~/Downloads$ grep FAIL v0.9.1-results.csv |wc -l
7

It gives me 805.

If we are considering 800 all together for all variants, that becomes relatively easy to overcome by introducing more variants. MSI has at least 2 variants (DDR4 and DDR5). If we count WIFI and no-WIFI, we have 4 variants. One variant needs barely 200 tests (including skips and TBDs?). That's... at least not serious.

MSI will not pass the second condition.

Unless I don't udnerstand this requirement and we mean all executed tests for releases. Do RC tests also count? Then we could probably qualify very quickly...

Yes, because we have a lot of tests, at least for those platforms. Does all platforms can run 800+ test cases?
Also, for platforms without multiple variants (read multiple target audiences).

IMHO, the key is time on the market and the volume of devices. The pass rate is essential, but only when it is associated with a certain number of tests. If we have ten tests, then a 100% pass rate is not good enough, IMHO.

OTOH contribution is welcome; please provide suggestions on how you would classify.

@miczyg1
Copy link
Contributor

miczyg1 commented Apr 10, 2025

Yes, because we have a lot of tests, at least for those platforms. Does all platforms can run 800+ test cases?
Also, for platforms without multiple variants (read multiple target audiences).

But still this doubles the test cases because we have two variants (with close to identical mainboard). To me it is like cheating...

What if releases are done separately? For NV4x and NS5x it was true. Then each would have to count separately.

From the PR:

Historical Foundation: There have been at least two releases targeting this
platform or one of its variants, targeting diverse user bases, over a span of
no fewer than five months.

Can we consider dGPU and iGPU variants of the same platform? Each of these had only one release (v0.9.1 and v0.9.0 respectively). If they are the same platform, then iGPU should have received v0.9.1 too. If iGPU received v0.9.1, then it probably would qualify for number of releases. But it didn't happen.

To me iGPU and dGPU are different platforms, because we treated them as such historically. So none of them qualify for the Historical Foundation.

Copy link
Contributor

@macpijan macpijan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't approve it without @pietrushnic consent

We had one from @pietrushnic as requested.

We can contribute to the guidelines in documentation if they need to be further adjusted.

@mkopec mkopec merged commit 8c87648 into dasharo Apr 10, 2025
54 of 55 checks passed
@mkopec mkopec deleted the ncm_rc_100 branch April 10, 2025 16:07
@macpijan
Copy link
Contributor

macpijan commented Apr 10, 2025

OTOH, we already have in this chapter:

Major version zero (0.y.z) is for initial development or first release issued
and may not support all Dasharo Quality Criteria.

So one can imply for this that we can release major one version when it's not an initial development anymore, and is not limited in support.

By hearing how peculiar bugs are we chasing already (meaning the base foundation is in place), and the planned features for this release, I would say it's far from initial development.

We can keep arguing whether it should be 200 or 400 tests or how it should be counted, but I am not sure how valuable this is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants