Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Renamed preferences key from "brackets-themes" to "themes" to avoid conf... #8475

Merged
merged 1 commit into from
Jul 21, 2014
Merged

Renamed preferences key from "brackets-themes" to "themes" to avoid conf... #8475

merged 1 commit into from
Jul 21, 2014

Conversation

MiguelCastillo
Copy link
Contributor

...lict with the themes extension
#8461

@MiguelCastillo
Copy link
Contributor Author

cc @dangoor

@TomMalbran
Copy link
Contributor

@MiguelCastillo If the font preferences are no longer under the theme prefix, which preferences are left under it? Do we really need to have a themes prefix or we can just remove it since is a core feature?

@MiguelCastillo
Copy link
Contributor Author

@TomMalbran I left themes in there because there was no real objection one way or the other from @dangoor when we had a short discussion here #8461.

But if we all think is better to remove the prefix, then let's do it! My preference has always been to scope things in a namespace type of way... Kinda makes it clearer to see what code is doing what.

Fonts settings are still going under that, but the fonts api will have its own "fonts" prefix. Unless of course we don't want that there either.

@dangoor, any preference?

@lkcampbell
Copy link
Contributor

@MiguelCastillo, this comment isn't directly related to this PR, but this looks as good of a place as any to bring this up. Do you need to change the license text at the beginning of these files to match the Adobe licensing text?

@dangoor
Copy link
Contributor

dangoor commented Jul 21, 2014

We still have two settings: the selected theme and customScrollbars, so I think putting those under a "themes" prefix is fine.

@lkcampbell The license blocks are fine, I think, since we don't explicitly require copyright assignment (nor is it really necessary with the MIT license).

dangoor added a commit that referenced this pull request Jul 21, 2014
Renamed preferences key from "brackets-themes" to "themes" to avoid conf...
@dangoor dangoor merged commit ef13232 into adobe:master Jul 21, 2014
@dangoor
Copy link
Contributor

dangoor commented Jul 21, 2014

Looks fine. Merging

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants