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

Simplify the overriding of Trepn preferences #34

Conversation

sshann
Copy link

@sshann sshann commented Aug 28, 2020

This commit extends the overriding logic of Trepn preferences into a self-contained function, allowing users to override all configurations. Mirror the preferences key in the user-defined configuration to extend the overriding to any preference. To reduce the verbosity of the key, the mirrored key contains only the substring after the last dot. For instance, the preference com.quicinc.preferences.general.profiling_interval is mapped into profiling_interval.

Comment on lines +196 to +190
row = {key: value + float(new[key]) for key, value in list(accum.items()) if
key not in ['Component', 'count']}
Copy link
Author

Choose a reason for hiding this comment

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

This and the change below were autoformatted due to the lines being so long.
If prefered, I can undo these changes.

@sshann sshann force-pushed the allow_setting_custom_preferences_in_trepn_profiler branch from c2c37a7 to 2e18a15 Compare August 28, 2020 12:22
@sshann sshann marked this pull request as draft August 31, 2020 16:52
@sshann
Copy link
Author

sshann commented Aug 31, 2020

I am doing a small change to allow to automatically override the remaining preferences without further changes in the source code

This commit extends the overriding logic of Trepn preferences into a self-contained function, allowing users to override all configurations. Mirror the preferences key in the user-defined configuration to extend the overriding to any preference. To reduce the verbosity of the key, the mirrored key contains only the substring after the last dot. For instance, the preference `com.quicinc.preferences.general.profiling_interval` is mapped into `profiling_interval`.
@sshann sshann force-pushed the allow_setting_custom_preferences_in_trepn_profiler branch from 41d4ad3 to 2ee5e3c Compare August 31, 2020 17:53
@sshann sshann marked this pull request as ready for review August 31, 2020 17:55
@EricZielinski
Copy link
Collaborator

Looks good, but can you also update the relevant example configurations that use Trepn in examples/?

@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sshann
Copy link
Author

sshann commented Sep 1, 2020

Native and web examples are updated.
I took the liberty to format the example files.

@sshann
Copy link
Author

sshann commented Sep 1, 2020

By the way, wouldn't it be nicer to have the Trepn and the Android profiler in their own directory?
I assume that they were the first and the original author decided to organize by native/web.
The profilers that were added afterwards are all in their own directory, so it makes sense to also update Trepn and Android to their own directory.

@EricZielinski
Copy link
Collaborator

By the way, wouldn't it be nicer to have the Trepn and the Android profiler in their own directory?
I assume that they were the first and the original author decided to organize by native/web.
The profilers that were added afterwards are all in their own directory, so it makes sense to also update Trepn and Android to their own directory.

Yes, I think you're right - they were the first. It makes sense to separate them.

@EricZielinski EricZielinski merged commit ab9e922 into S2-group:master Sep 1, 2020
@sshann sshann deleted the allow_setting_custom_preferences_in_trepn_profiler branch September 1, 2020 12:29
sshann added a commit to sshann/android-runner that referenced this pull request Sep 1, 2020
PR S2-group#34 updates the Trepn profiler and allows overriding any preference. This caused a unit test to break as the PR updates the configuration file and replaces `sampligin_profiling` attribute by a map of preferences. This should have been notified in the PR itself.
sshann added a commit to sshann/android-runner that referenced this pull request Sep 1, 2020
PR S2-group#34 updates the Trepn profiler and allows overriding any preference. This caused a unit test to break as the PR updates the configuration file and replaces `sample_interval` attribute by a map of preferences. This should have been notified in the PR itself.
sandervano pushed a commit to sandervano/android-runner that referenced this pull request Jan 21, 2022
…rences_in_trepn_profiler

Simplify the overriding of Trepn preferences
sandervano pushed a commit to sandervano/android-runner that referenced this pull request Jan 21, 2022
PR S2-group#34 updates the Trepn profiler and allows overriding any preference. This caused a unit test to break as the PR updates the configuration file and replaces `sample_interval` attribute by a map of preferences. This should have been notified in the PR itself.
HermanKelder pushed a commit to HermanKelder/GreenLabCSSPrefixes that referenced this pull request Oct 30, 2022
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.

2 participants