-
-
Notifications
You must be signed in to change notification settings - Fork 11
breaking: Clearer feature flags #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
✅ All contributors have signed the CLA |
Code Coverage Summary (test-no-branch.xml)Diff against mainResults for commit: 5409dc6 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Code Coverage Summary (togglable report)DetailsResults for commit: 5409dc6 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Code Coverage Summary (test-missing-lines.xml) without detailed coverageResults for commit: 5409dc6 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Code Coverage Summary (test-no-branch.xml) without detailed coverageDiff against mainResults for commit: 5409dc6 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Code Coverage Summary (comprehensive test)DetailsResults for commit: 5409dc6 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Code Coverage Summary (test-missing-lines.xml)Results for commit: 5409dc6 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
|
I have read the CLA Document and I hereby sign the CLA |
|
I'll test this later tonight, I'll update here once it's done. Thanks a lot! |
|
Hey @cicdguy, sorry for the wait. I've tested the changes and found a small bug. When extracting the overall coverage rate, we're parsing it from the last value from a line such as: However, since we're directly parsing it from the truncated rate ( That way, even an untested one-liner PR gets caught in the |
Co-authored-by: Emmanuel <62708624+emmanuelsdias@users.noreply.github.com> Signed-off-by: cicdguy <26552821+cicdguy@users.noreply.github.com>
|
@emmanuelsdias Thanks again for the wonderful suggestions! If all the testing looks good on your end, I will merge this and create a major release. Let me know :) |
|
LGTM, thanks a lot! |
Addresses comments from #45 (comment)
Hi @emmanuelsdias -are you able to test these new features on your workflows? You can use the branch instead of the version tag (breaking/clearer-feature-flags)
Once we test this on our end and if it looks good on your end, we can create a major release out of this (since the "legacy" feature flags will be retained in the older major release).