Skip to content

Conversation

@dantleech
Copy link
Member

  • Enable setting expansion for timings
  • Enableable timing for query / traversal
  • Config is automatically based with dist config

Copy link
Member Author

Choose a reason for hiding this comment

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

Should probably use the stopwatch component if I do much more of this.

Copy link
Member

Choose a reason for hiding this comment

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

i would leave it as is for now and add StopWatch if you have more cases.

@dantleech dantleech force-pushed the config_exutiontime branch 3 times, most recently from af35888 to 79996c8 Compare December 19, 2014 10:02
Copy link
Member

Choose a reason for hiding this comment

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

could it make sense to use OptionsResolver instead of this class? the cool thing about it is that you get validation and defaults handling for free.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I would still need this class to represent the instance of the configuration, but yeah we could integrate the OptionResolver

tbh the current config system is still a bit messy/strange and could use a general refactor.

Copy link
Member Author

Choose a reason for hiding this comment

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

- Enable setting expansion for timings
- Enableable timing for query / traversal
- Config is automatically based with dist config
dantleech added a commit that referenced this pull request Dec 19, 2014
@dantleech dantleech merged commit 659b067 into master Dec 19, 2014
@dantleech dantleech deleted the config_exutiontime branch December 19, 2014 10:38
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.

4 participants