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 more menu functions (flash lights and template portions in code) #13

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

n-gineer
Copy link

I wanted to enable adding all the rest of the app functions (or at least the simple commands) via a set of template changes, and started with the "Flash Lights" command. Can you please try it out and let me know if it is good to move on with the rest?

After starting this, I noticed you already have a pull request in place for open port #10 (which is arguably the most important one). I'd be happy to incorporate that pull request into my own development as well.

@srwalter
Copy link
Owner

Overall looks good. Would you mind to squash the first several commits together? I think there should just be two commits, adding the template comments, and then adding flash_lights. I also noticed a couple whitespace errors where things didn't seem to align (could be tabs versus spaces)

@n-gineer
Copy link
Author

n-gineer commented Apr 2, 2021

I don't have much practice with squashing but will try to figure it out and get that updated, as well as looking for white space errors.

Test comment in .mc file

Fix bad comment test; add template command 1/6

I was wrong, this isn't a configuration file but a my class file, with standard C comments (Monkey C to be exact) https://developer.garmin.com/connect-iq/reference-guides/monkey-c-reference/

add template command 2/6

add template command 3/6

add template command 4/6

add template command 5/6

add template command 6/6
@n-gineer n-gineer force-pushed the add_more_menu_functions branch from 7826f25 to 323bf8e Compare October 30, 2021 20:07
@n-gineer
Copy link
Author

I finally got around to figuring out squashing. However, It is showing this now has conflicts, but I can't even find a way to view the conflicts, much less find a way to resolve them.

@srwalter
Copy link
Owner

Try running this locally: "git fetch origin && git rebase origin/master"

@paulobrien
Copy link
Contributor

paulobrien commented Oct 30, 2021 via email

@n-gineer
Copy link
Author

Try running this locally: "git fetch origin && git rebase origin/master"

resulted in:

Resolve all conflicts manually, mark them as resolved with                                                              
"git add/rm <conflicted_files>", then run "git rebase --continue".                                                      
You can instead skip this commit: run "git rebase --skip".                                                              
To abort and get back to the state before "git rebase", run "git rebase --abort".                                       
Could not apply 687048e... Fix bad comment test; add template command 1/6                                               
Auto-merging source/MainDelegate.mc                                                                                     
CONFLICT (content): Merge conflict in source/MainDelegate.mc                                                                                                                              ````



@paulobrien
Copy link
Contributor

paulobrien commented Oct 30, 2021 via email

@n-gineer
Copy link
Author

It's more complex than that, leave it with me. P

will do; sorry!

Copy link
Author

@n-gineer n-gineer left a comment

Choose a reason for hiding this comment

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

minor edit recommended to fix my typographical mistake

@@ -25,6 +25,8 @@ class SecondDelegate extends Ui.BehaviorDelegate {
var _unlock;
var _lock;
var _settings;
// Template: // var _%lower_case% dnl
Copy link
Author

Choose a reason for hiding this comment

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

"dnl" is a mistake from testing and should be deleted when this pull is merged.

@n-gineer
Copy link
Author

@paulobrien Any intention to implement this? Anything I can do to make it more useful?

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