GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Watch face selection using CMake by JF002 · Pull Request #1934 · InfiniTimeOrg/InfiniTime

Conversation

@JF002

@JF002 JF002 commented Dec 24, 2023

Copy link
Copy Markdown
Collaborator

The list of watch face to build into the firmware is now set by CMake (-DENABLE_WATCHFACES).

Fix SettingWatchFace : convert to index to/from WatchFace when needed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@JF002 JF002 added this to the 1.14.0 milestone Dec 24, 2023
@github-actions

github-actions Bot commented Dec 24, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Build size and comparison to main:

Section Size Difference
text 369768B 208B
data 940B 0B
bss 63516B 0B
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JF002
Watch face selection using CMake
14d002d 
The list of watch face to build into the firmware is now set by CMake (-DENABLE_WATCHFACES).

Fix SettingWatchFace : convert to index to/from WatchFace when needed.
@JF002
JF002 force-pushed the cmake-watchface-selection branch from 148e275 to 14d002d Compare December 24, 2023 11:03
@JF002
Watch face selection using CMake
bf56387 
Update Apps.md to mention the selection of watchfaces using Cmake.
@vkareh

vkareh commented Jan 5, 2024

Copy link
Copy Markdown
Contributor

This works well for me (I rebased this locally against main). Disabling some of the faces I'm not personally interested in shaved off 23K from my image file as compared to main. That's a huge deal :)
JF002 and IdeallyGrey reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
NeroBurner
NeroBurner requested changes Jan 6, 2024
View reviewed changes

@NeroBurner NeroBurner left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

watchface to watch face, some wording and a small cmake readability improvement

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/displayapp/apps/CMakeLists.txt Resolved

Uh oh!

There was an error while loading. Please reload this page.
@JF002 @NeroBurner
CMake watch faces selection
ad8e801 
Improve wording and replace "watchface" by "watch face" in Apps.md.
Improve CMake readability regarding watch face selection

Co-authored-by: Reinhold Gschweicher <[email protected]>
@JF002

JF002 commented Jan 6, 2024

Copy link
Copy Markdown
Collaborator Author

@vkareh Thanks for the feedback! That was exactly the goal of those changes :+1

@NeroBurner Thanks for the review, I applied all your suggestions !
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
FintasticMan
FintasticMan approved these changes Jan 6, 2024
View reviewed changes
NeroBurner
NeroBurner approved these changes Jan 6, 2024
View reviewed changes

@NeroBurner NeroBurner left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
FintasticMan
FintasticMan requested changes Jan 6, 2024
View reviewed changes

@FintasticMan FintasticMan left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Almost missed this!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/code/Apps.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@JF002 @FintasticMan
CMake watch faces selection
5cfb956 
Documentation : watch faces are not system apps anymore.

Co-authored-by: FintasticMan <[email protected]>
FintasticMan
FintasticMan approved these changes Jan 6, 2024
View reviewed changes
@JF002
JF002 merged commit 6505336 into main Jan 6, 2024
@JF002
JF002 deleted the cmake-watchface-selection branch January 6, 2024 13:44
@JF002 JF002 mentioned this pull request Mar 19, 2024
[WIP/RFC] Remove watchface enum #2040
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NeroBurner NeroBurner NeroBurner approved these changes
@FintasticMan FintasticMan FintasticMan approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.14.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@JF002 @vkareh @NeroBurner @FintasticMan
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.