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

Add flag to disable/enable boldness #99

Merged
merged 10 commits into from
Oct 23, 2019
Merged

Add flag to disable/enable boldness #99

merged 10 commits into from
Oct 23, 2019

Conversation

ccmetz
Copy link
Contributor

@ccmetz ccmetz commented Oct 20, 2019

Addresses issue #95

This is my first time using Rust so let me know if there is anything that I'm doing that isn't following best practices. 🙂

@ccmetz
Copy link
Contributor Author

ccmetz commented Oct 20, 2019

Looks like there are some merge conflicts from some recent work on master. I should be able to work on addressing these later today.

@o2sh o2sh mentioned this pull request Oct 20, 2019
@ccmetz
Copy link
Contributor Author

ccmetz commented Oct 20, 2019

I fixed the merge conflicts but I still need to make adjustments to existing tests

@o2sh o2sh requested review from andymac-2 and spenserblack and removed request for andymac-2 and spenserblack October 21, 2019 07:40
@ccmetz
Copy link
Contributor Author

ccmetz commented Oct 22, 2019

Just noticed I have a few more conflicts to resolve! I can work on resolving those tomorrow.

src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@spenserblack
Copy link
Collaborator

Sorry for the late review, just realized I didn't submit it for 2 days 🤦‍♂️

@ccmetz
Copy link
Contributor Author

ccmetz commented Oct 23, 2019

The latest commits include some adjustments based on the new file organization as well as changing the command line option from --bold <on|off> to --no-bold.

Copy link
Collaborator

@spenserblack spenserblack left a comment

Choose a reason for hiding this comment

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

Good work, @ccmetz! Just a few more recommended changes.

src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@ccmetz
Copy link
Contributor Author

ccmetz commented Oct 23, 2019

Just pushed some changes that address the most recent code review comments

Copy link
Collaborator

@spenserblack spenserblack left a comment

Choose a reason for hiding this comment

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

🎉

@o2sh o2sh merged commit 793bc43 into o2sh:master Oct 23, 2019
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.

3 participants