#define AUTO_SHIFT_SETUP

[ThePreviousOne]

Description

I added a config option called AUTO_SHIFT_SETUP for autoshift_timer_report autoshift_enable autoshift_disable funtions and KC_ASRP KC_ASUP KC_ASDN keycodes

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[zvecr]

I would say the logic of AUTO_SHIFT_SETUP reads badly to what it actually does and should really be renamed. Naming things is always difficult, but its closer to AUTO_SHIFT_NO_SETUP.

Also should we be stripping out the enable/disable functions?

[ThePreviousOne]

my bad I thought I used #ifdef not #ifndef, it is still tested and does work
is there a preference?

Yes they should, the serve no purpose other than to have dedicated on off keys, I left toggle for that

[ThePreviousOne]

I missed the keycodes, force pushed to avoid unnecessary commits in the history

[zvecr]

I would rather keep the old behaviour as default so we are not silently breaking existing users workflow. Without maintaining the existing contract, it will need to go through the breaking change process.

Going with something like AUTO_SHIFT_NO_SETUP allows us to avoid that.

[ThePreviousOne]

I'm only 'removing' the report function can't imagine anyone using that on daily basis, But I get your point. Sent from ProtonMail mobile

…

-------- Original Message --------

On Mar. 16, 2020, 2:38 p.m., Joel Challis wrote: I would rather keep the old behaviour as default so we are not silently breaking existing users workflow. Without maintaining the existing contract, it will need to go through the breaking change process. Going with something like AUTO_SHIFT_NO_SETUP allows us to avoid that. — You are receiving this because you authored the thread. Reply to this email directly, [view it on GitHub](#8441 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ADKKNX3DA2GMSSBGXGALNM3RHZW3FANCNFSM4LLTKN7Q).

[ThePreviousOne]

I just made the change and went to reflect it in the docs and found Remove AUTO_SHIFT_SETUP from your config.h. on line 142
I didn't add it, it's already there. It's just not in the code.

Am I implementing a feature that has supposed to have existed for a while now?

[ThePreviousOne]

@zvecr its no longer a breaking change

[tzarc]

Retargeted to develop due to core changes.

[noroadsleft]

Slight __attribute__ ((weak)) ✔️ as I don't use Auto Shift.

[noroadsleft]

Thanks!

For future reference, we recommend against committing to your master branch as you've done here, because pull requests from modified master branches can make it more difficult to keep your QMK fork updated. It is highly recommended for QMK development – regardless of what is being done or where – to keep your master updated, but NEVER commit to it. Instead, do all your changes in a branch (branches are basically free in Git) and issue PRs from your branches when you're developing.

There are instructions on how to keep your fork updated here:

Best Practices: Your Fork's Master: Update Often, Commit Never

Fixing Your Branch will walk you through fixing up your master branch moving forward. If you need any help with this just ask.

Thanks for contributing!

[ThePreviousOne]

This wasn't technically on my master branch I was using a seperate instance in a different location. because at the time I had implemented alot of small breaking changes do to memory constraints in my controller.