Skip to content
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

Revamp and C++ify notes #252

Merged
merged 3 commits into from
Jan 2, 2021
Merged

Revamp and C++ify notes #252

merged 3 commits into from
Jan 2, 2021

Conversation

mattgodbolt
Copy link
Owner

  • All C++ all the time
  • Mostly global-free. Only one singleton bridging to Xania
  • Tests!

Still writes to area/notes.txt which means this does not help with #221,
but at least we can test the fix as and when.

* All C++ all the time
* Mostly global-free. Only one singleton bridging to Xania
* Tests!

Still writes to area/notes.txt which means this does not help with #221,
but at least we can test the fix as and when.
@mattgodbolt mattgodbolt requested a review from snellers December 31, 2020 18:55
@mattgodbolt
Copy link
Owner Author

Some notes:

  • times are now formatted as gmtime else my tests fail (they were in "local" time where local is Chicago for me...)
  • some minor tweaks to a few other routines

This is so low priority so don't fret. If I don't hear in a week or so I might merge anyway unless you have a strong feeling you want to look. This was more to get me back into Xania and C++ :)

src/Note.cpp Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
src/Note.cpp Outdated Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
src/Note.cpp Show resolved Hide resolved
@snellers
Copy link
Collaborator

Looks nice!

@mattgodbolt
Copy link
Owner Author

Well, one of the test hosts died with:

[-325297776] TheMoog: So here it is
..."
 ==
"
[  0] TheMoog: So here it is
...

so...time to work out what travesty I've managed to slip past the regular tests...

I suspect I know actually, and it's something I meant to change! Watch this spaacespace.

@mattgodbolt
Copy link
Owner Author

Thanks for the comprehensive review btw! :)

@mattgodbolt mattgodbolt merged commit 41b9987 into main Jan 2, 2021
@mattgodbolt mattgodbolt deleted the mg/revamp_notes branch January 2, 2021 17:18
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.

2 participants