Skip to content

Conversation

@NoobTracker
Copy link

Circles created with fillCircle() were one pixel taller than wide.

Circles created with fillCircle() were one pixel taller than wide.
@marcelstoer
Copy link
Member

Great to see this PR, well done!

However, you moved the .cpp file out of src/ into the root folder instead of editing the existing file. Please fix that in your circles-fixed branch and commit the change.

@NoobTracker
Copy link
Author

Oh, sorry. I'll fix that.

@NoobTracker
Copy link
Author

Okay, done.

@marcelstoer
Copy link
Member

Check the "Files changed" tab: https://github.com/ThingPulse/esp8266-oled-ssd1306/pull/279/files

If you changed a single line then we would expect to see a single line in the diff.

@NoobTracker
Copy link
Author

Ah, I've added a line (drawVerticalLine, line ~300) and maybe removed an empty line.

@NoobTracker
Copy link
Author

I don't now how GitHub handles that. I only changed fillCircle().

@NoobTracker
Copy link
Author

Yes, it looks like that's the reason.

@NoobTracker
Copy link
Author

Ooops, the actual reason is that I downloaded the files and replaced them on GitHub.

@marcelstoer
Copy link
Member

You created a bit of a mess here because A) this one is "broken" as you realized and B) you were branching your other two branches off of this one rather than master. Hence, we see these commit in the other PRs, too.

Here's my proposal for a way forward:

  • fix this PR by e.g.
    • git checkout circles-fixed
    • git reset --hard origin/master // you will loose all local changes!
    • change that one(?) line that needs fixing
    • git commit -am "whatever message"
    • git push --force
  • then EITHER wait for this PR to land and proceed likewise for the other branches
  • OR proceed with the other branches right now and deal with potential merge conflicts later

@NoobTracker
Copy link
Author

The problem with this is that the newer version with the ring functions all changes the circular functions. In addition, the header file must also change. When I try to do this on GitHub, it is much harder to test the program. I would have to download it then. And the latest version (keywords) contains all changes to circles-fixed and circles-improved-rings. And where can I use this command line?

@NoobTracker NoobTracker deleted the circles-fixed branch May 11, 2020 12:22
@marcelstoer marcelstoer mentioned this pull request Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants