Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Removed explicit check for true/false #7036

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

Closed
arnavb wants to merge 1 commit into arduino:master from arnavb:master
Closed

Removed explicit check for true/false #7036

arnavb wants to merge 1 commit into arduino:master from arnavb:master

Conversation

Copy link

@arnavb arnavb commented Dec 19, 2017

The title says it all.

Copy link
Collaborator

@matthijskooijman matthijskooijman left a comment

Choose a reason for hiding this comment

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

Agreed, there's absolutely no reason to specify this explicitly. This should not affect the generated code in any way, so it seems safe to merge.

This change probably also needs to be applied to the SAM and SAMD cores, though.

Copy link
Author

arnavb commented Dec 19, 2017
edited
Loading

Ok, should I make the changes on those files in this PR? (I can't seem to find the files, if you want me to make the changes, tell me where)

Copy link
Collaborator

The other cores live in different repositories, so they'd need different PRs.

@facchinm, @cmaglie, what's the plan for these shared files? I remember some plans to extract them into a shared space, but I'm not sure how far along those plans are? Regardless, what's the best course to take here? Preparing separate PRs, or just copying over the entire Stream.cpp file at some later point?

arnavb reacted with thumbs up emoji

@facchinm facchinm added Print and Stream class The Arduino core library's Print and Stream classes feature request A request to make an enhancement (not a bug fix) labels Dec 20, 2017
Copy link
Author

arnavb commented Feb 26, 2018
edited
Loading

@matthijskooijman You said this was safe to merge a few months ago. Any chance of this actually happening? This is a minor change, so it shouldn't affect much. (Sorry to pester you)

Copy link
Collaborator

@arnavb, I'm not in a position to merge things, I usually only review stuff. You'll have to pester @cmaglie and @facchinm instead :-p

Copy link
Author

arnavb commented Feb 26, 2018

Well, since you've pinged them, that's good.

Copy link
Member

facchinm commented Mar 2, 2018

Hi @arnavb , sure, we are in the middle of a transition to https://github.com/arduino/ArduinoCore-avr so I'd prefer not to merge the PR here but to merge it there. Furthermore, as you can see from the "Print and Stream class" tag, it is not an hardware related change so it's going to be merged as part of Chainsaw project. Sorry for the confusion about all these repos but we are trying to provide on a more modern way to contribute to the project.

arnavb reacted with thumbs up emoji

Copy link
Author

arnavb commented Mar 2, 2018

@facchinm Makes sense! Thanks for the info!

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

@matthijskooijman matthijskooijman matthijskooijman approved these changes

Assignees
No one assigned
Labels
feature request A request to make an enhancement (not a bug fix) Print and Stream class The Arduino core library's Print and Stream classes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /