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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃敟 remove implementation of validate themes #7490

Merged
merged 1 commit into from Oct 5, 2016

Conversation

Projects
None yet
2 participants
@kirrg001
Contributor

kirrg001 commented Oct 5, 2016

no issue!

I just did this fast tidy up PR, took me 2mins.
Theme validation is handled by gscan!
So validation happens on upload via Ghost-Admin.

@ErisDS When people copying their theme manual into the content folder, should we create another PR which validates these themes via gscan?

馃敟 remove implementation of validate themes
no issue
- theme validation is handled by gscan
@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Oct 5, 2016

Member

Yisss! 馃帀
Definitely let's remove this.

There are some things that need changing in how gscan is structured, so that we definitely can treat fatal errors differently to warnings.

Adding (installing) a theme, should probably only check that what's being installed IS a theme (which will get easier with the new package.json requirements).

I think there will eventually be some sort of interface that shows themes, along with if there's an update and if it's safe to activate. And/or theme activation may need an extra check for fatal errors.

I think once the CLI is in use, the manual copy into content folder use-case should fall into edge-case-land. We'll possibly leave it unchecked, and if users run into problems, add checks back later.

All of this should come a lot clearer when I've made some more progress on #7491.

For now... this, definitely this!

Member

ErisDS commented Oct 5, 2016

Yisss! 馃帀
Definitely let's remove this.

There are some things that need changing in how gscan is structured, so that we definitely can treat fatal errors differently to warnings.

Adding (installing) a theme, should probably only check that what's being installed IS a theme (which will get easier with the new package.json requirements).

I think there will eventually be some sort of interface that shows themes, along with if there's an update and if it's safe to activate. And/or theme activation may need an extra check for fatal errors.

I think once the CLI is in use, the manual copy into content folder use-case should fall into edge-case-land. We'll possibly leave it unchecked, and if users run into problems, add checks back later.

All of this should come a lot clearer when I've made some more progress on #7491.

For now... this, definitely this!

@ErisDS ErisDS merged commit c4e47c9 into TryGhost:master Oct 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

mixonic added a commit to mixonic/Ghost that referenced this pull request Oct 28, 2016

馃敟 remove implementation of validate themes (TryGhost#7490)
no issue

- theme validation is handled by gscan

madfrog2047 added a commit to madfrog2047/Ghost that referenced this pull request Nov 20, 2016

馃敟 remove implementation of validate themes (TryGhost#7490)
no issue

- theme validation is handled by gscan
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment