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

Move Logs page JS to appropriate location #10265

Merged
merged 3 commits into from Sep 10, 2018

Conversation

Projects
None yet
6 participants
@sarjon
Member

sarjon commented Sep 3, 2018

Questions Answers
Branch? develop
Description? Seems that new-theme/js/app contains only Vue related code. This PR moves Logs page js to new-theme/js/pages where all other page specific js code lives.
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? n/a
How to test? Logs page works the same

This change is Reviewable

@LouiseBonnard

This comment has been minimized.

Show comment
Hide comment
@LouiseBonnard

LouiseBonnard Sep 6, 2018

Contributor

@sarjon, is there any translation key to check here? I can't find any.

Contributor

LouiseBonnard commented Sep 6, 2018

@sarjon, is there any translation key to check here? I can't find any.

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 6, 2018

Member

@LouiseBonnard there arent any, @prestonBot is lying.

Member

sarjon commented Sep 6, 2018

@LouiseBonnard there arent any, @prestonBot is lying.

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Sep 10, 2018

Member

I wonder if the vuejs pages should not be merged with the other pages, I was lost when I try to find my way in these folders.

Anyway, can you please rebase? :)

Member

Quetzacoalt91 commented Sep 10, 2018

I wonder if the vuejs pages should not be merged with the other pages, I was lost when I try to find my way in these folders.

Anyway, can you please rebase? :)

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 10, 2018

Member

@Quetzacoalt91 rebased. 👍

Member

sarjon commented Sep 10, 2018

@Quetzacoalt91 rebased. 👍

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Sep 10, 2018

Member

Are you sure? I still see a conflicted file. :)

Member

Quetzacoalt91 commented Sep 10, 2018

Are you sure? I still see a conflicted file. :)

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 10, 2018

Member

oh god, those assets are so annoying. 😑

Member

sarjon commented Sep 10, 2018

oh god, those assets are so annoying. 😑

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 10, 2018

Member

rebased (for now). 👍

Member

sarjon commented Sep 10, 2018

rebased (for now). 👍

@ntiepresta ntiepresta added QA ✔️ and removed waiting for QA labels Sep 10, 2018

@Quetzacoalt91 Quetzacoalt91 merged commit 6acb13a into PrestaShop:develop Sep 10, 2018

2 of 3 checks passed

code-review/reviewable 25 files left
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91
Member

Quetzacoalt91 commented Sep 10, 2018

Thank you @sarjon

@PierreRambaud PierreRambaud added this to the 1.7.5.0 milestone Oct 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment