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

M43 - add Toggle utility and Z servo utility (replaces PR #5864) #6219

Merged
merged 1 commit into from
Apr 6, 2017
Merged

M43 - add Toggle utility and Z servo utility (replaces PR #5864) #6219

merged 1 commit into from
Apr 6, 2017

Conversation

Bob-the-Kuhn
Copy link
Contributor

Ran into rebase issues with PR #5864 so killed it and opened this one.

No changes from PR #5864.

@thinkyhead
Copy link
Member

thinkyhead commented Apr 3, 2017

Note my comment on the other PR. Even if you start a new branch, you can easily make the original PR point at those commits.

I'll review this a little later.

@Roxy-3D
Copy link
Member

Roxy-3D commented Apr 3, 2017

Ran into rebase issues with PR #5864 so killed it and opened this one.

I know my method is very brute force... But it does work. When I can't get past the conflict stage, I've been able to save my directory of files. Then I delete the repository from my GitHub account. I re-fork. And then add the depository to GitHub Desktop and clone into a new directory. I suspect I don't have to Sync but I've been doing a Sync. And then I copy any file that changed to the new clone directory. I do a commit and Sync. And then I can go to my GitHub page and issue a Pull Request.

I've been hurrying up and merging just because I don't want another change to show up and cause yet another conflict. I suspect that is not necessary. My guess is because I was first in line, I don't get the conflict message. But I don't know that for a fact so I've been doing a quick merge just to get out of way of more problems.

@Bob-the-Kuhn Bob-the-Kuhn merged commit a5abc61 into MarlinFirmware:RCBugFix Apr 6, 2017
@thinkyhead
Copy link
Member

thinkyhead commented Apr 11, 2017

Then I delete the repository from my GitHub account.

You should never need to do this. Study, study, study!
Get to know the tools and you won't have to keep fighting with them.

@thinkyhead
Copy link
Member

thinkyhead commented Apr 11, 2017

@Bob-the-Kuhn This was merged a little too quickly.

I prefer to scan all PRs for coding standards and to make sure everything is in compliance with The Marlin Way. Having done this job daily for 2 years, I can very quickly apply formatting, catch errors, reduce code size, and so on. One thing that should have been added is an entry in .travis.yml to test PINS_DEBUGGING. That would have caught some of the errors that I'm finding now.

For example, you can't do this in C++:

#if !defined(z_servo_angle)

It should have been:

#if !HAS_Z_SERVO_ENDSTOP

So, please allow me to clean up your commits before merging them.

@Bob-the-Kuhn
Copy link
Contributor Author

Will do.

Sorry about that.

@thinkyhead
Copy link
Member

thinkyhead commented Apr 12, 2017

Sorry about that.

You're doing good stuff, generally-speaking, so no worries. I know it's been a mad rush to merge patches lately, especially bug-fixes, and when I do these 2-day rounds without sleep I make stupid errors. But my flawed code sure looks pretty….

Anyway, you can probably tell by the types of issues cropping up lately that Marlin is getting very close to release. Most of the bugs reported are subtle, and in general we're not seeing anything too flaky. There will be a round of fixes after release, I'm sure, but I think we got all the ugliest bugs.

I would like to spend the next 2 weeks:

  • polishing up the most recent additions,
  • briefing configuration tool writer(s) on the latest options,
  • adding to the documentation, and
  • reviewing PRs for: merge, 1.1.1 milestone, or 1.2.0 milestone

As to that last one, I've marked several PRs for these milestones as part of the feature-freeze, and for the next 20 days we should accept only patches that are innocuous, obvious, cleanup, documentation, or reparative.

I would also like to improve our flow by:

  • recruiting help willing to field bug reports, fix issues, and prep PRs for merge, and
  • bringing the core developers up to speed on the workflow and toolset.

That last bullet-point is going to be essential for the next round, 1.2.0-dev, which will begin with a much longed-for reorganization of the code, followed by a big push to get the Hardware Access Layer into proper shape so we can target 32-bit boards. I will be studying that HAL code in earnest in the meantime.

@Bob-the-Kuhn Bob-the-Kuhn deleted the Servo-probe-and-toggle-pins branch July 29, 2017 16:46
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.

None yet

3 participants