musselwhizzle / Focus-Points

Plugin for Lightroom to show which focus point was active in the camera when a photo was taken
Apache License 2.0
345 stars 42 forks source link

Expand the logger #71

Closed musselwhizzle closed 7 years ago

musselwhizzle commented 7 years ago

Currently the app only logs as logger.warn. make 4 methods logVerbose, logInfo, logWarn, logError and set a logLevel property as global.

logLevel = 1
logWarn(message)
   if (logLevel >= 3) then
      logger.warn(message)
   end
end

if would be nice if logLevel was controlled by the plugin manager so clients could choose their log levels.

rderimay commented 7 years ago

I like that yes, this is more flexible. I think that would be a good idea to add a "grouping" parameter Instead of log(str) we could have log(group, level, str) which would output

EXIF      | Error
FUJIDELE  | Error

instead of the current just

Error
Error
musselwhizzle commented 7 years ago

sure, i would just say, separate the log levels to different methods; 1 for each level. It's more common such as in LrLogger and Android Log

logInfo(method, message)
...
logError(method, message)
rderimay commented 7 years ago

yep

musselwhizzle commented 7 years ago

yours.

reference: https://github.com/musselwhizzle/Focus-Points/tree/feature/prefs_pane

musselwhizzle commented 7 years ago

due to having so many feature branches out right now, please dont refactor all log statements just yet. It will make it harder to merge the conflicts. Just setup the feature/code and we'll do another pass for moving from log(str) to the new code once merges are in.

rderimay commented 7 years ago

Yes, understood.

rderimay commented 7 years ago
@musselwhizzle , is there a way to create a new branch based on another ? I would like to create a new branch for my log levels stuff building on the branch feature/prefs_pane. Is this possible easily ?
musselwhizzle commented 7 years ago
git fetch -- gets all the latest branch names from your remote repo
git checkout branch_name -- checks out an existing branch
git checkout -b branch_name - creates a new branch based off of where you currently are
git push -u origin branch_name - sends new local branch to remote repo (your personal fork) and sets up tracking

checkout whatever branch you want, probably this

git fetch
git checkout feature/fix_display_rotation
git pull
git checkout -b feature/your_new_feature
-- do work
-- add
git push -u origin feature/your_new_feature

you can merge in and out of branches easily if works is separated by feature.

rderimay commented 7 years ago

I continue to use Tower for my git, and reading it from the command line seems really totally simple. Should probably ditch Tower for Terminal..

Thanks !

On 11 janvier 2017 at 22:46:33, Joshua Musselwhite (notifications@github.com) wrote:

git fetch -- gets all the latest branch names from your remote repo git checkout branch_name -- checks out an existing branch git checkout -b branch_name - creates a new branch based off of where you currently are git push -u origin branch_name - sends new branch to remote and sets up tracking checkout whatever branch you want, probably this

git fetch git checkout feature/fix_display_rotation git pull git checkout -b feature/your_new_feature -- do work -- add git push -u origin feature/your_new_feature you can merge in and out of branches easily if works is separated by feature.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

rderimay commented 7 years ago

There is something weird still.

The PR #77 contained for example a file named FocusPointPrefs.lua which I cannot find in the branch prefs_pane you created.

Can you take a look what's going on ?

thx

On 11 janvier 2017 at 22:46:33, Joshua Musselwhite (notifications@github.com) wrote:

git fetch -- gets all the latest branch names from your remote repo git checkout branch_name -- checks out an existing branch git checkout -b branch_name - creates a new branch based off of where you currently are git push -u origin branch_name - sends new branch to remote and sets up tracking checkout whatever branch you want, probably this

git fetch git checkout feature/fix_display_rotation git pull git checkout -b feature/your_new_feature -- do work -- add git push -u origin feature/your_new_feature you can merge in and out of branches easily if works is separated by feature.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

musselwhizzle commented 7 years ago

Sorry, I changed it from master to feature/prefs_pane and then apparently didn't click merge but closed the PR. I merged it in now and can see the file here: https://github.com/musselwhizzle/Focus-Points/blob/feature/prefs_pane/focuspoints.lrdevplugin/FocusPointPrefs.lua

rderimay commented 7 years ago

Sorry, forget it. I think I misunderstood you. I thought you had merged the PR of the guy who did the pref panes example into the branch feature/prefs_pane. As I understand, you just did open the branch for me.

rderimay commented 7 years ago

ah ok, so we have it now !

rderimay commented 7 years ago

Solved by PR #85

musselwhizzle commented 7 years ago

requested a change in the PR, but merged anyway.