Skip to content

Conversation

pianistrevor
Copy link

@pianistrevor pianistrevor commented Feb 23, 2022

Highlights from CHANGELOG.md

  • See CHANGELOG.md for more

Issues Fixed

cerr << " " << (pass ? "" : "not ") << "ok " << ++mAssertCounter << " - " << description << endl;
if (!pass) {
if (pass) {
cout << " ok " << ++mAssertCounter << " - " << description << endl << flush;
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is off here.

cerr << description << " " << lhsLabel << " " << opLabel << " " << rhsLabel << endl;
if (!pass) {
if (pass) {
cout << " ok " << ++mAssertCounter << " - ";
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is off here.

- Update .gitattributes so we have consistent line endings
- Change 266 files from CRLF to LF.
- Run tests on push as well as on a pull request so developers can see impact
- Align value indicators in assertion statements
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I am being picky, but it would be great if you could split up the current commit (4bf8b53) into two separate commits, one for each entry here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you post a screenshot of the before/after alignment? I'm not sure what this change is addressing


#define assertEqualFloat(arg1, arg2, arg3) arduinoCIAssertOp("EqualFloat", "epsilon", arg3, compareMoreOrEqual, ">=", "actualDifference", fabs(arg1 - arg2))
#define assertNotEqualFloat(arg1, arg2, arg3) arduinoCIAssertOp("NotEqualFloat", "epsilon", arg3, compareLessOrEqual, "<=", "insufficientDifference", fabs(arg1 - arg2))
#define assertEqualFloat(arg1, arg2, arg3) arduinoCIAssertOp("EqualFloat", " epsilon", arg3, compareMoreOrEqual, ">=", "actualDifference", fabs(arg1 - arg2))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should either add some comments explaining how the extra spaces work, or possibly work around it entirely by making it part of the macro.

@ianfixes
Copy link
Collaborator

Please convert tabs to spaces in your code

@ianfixes
Copy link
Collaborator

Let's delay merging this until we get more discussion on #321. I'd also like some reassurance that cout << "blah" << flush is functionally equivalent (in terms of segfaults) to cerr << "blah".

I don't mean to be pedantic about this, but the original code I wrote use cout instead of cerr and I made the switch after troubleshooting unit tests that had segfaults in them: adafruit/Adafruit-WS2801-Library#20

In other words, cerr was the difference between seeing and not seeing the source of that segfault.

@pianistrevor
Copy link
Author

I think I'll take just the indentation and apply suggested changes in a new PR, while holding off on the cerr discussion.

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.

Output errors to stderr

3 participants