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

Refactor package directories #1265

Merged
merged 4 commits into from
Jun 30, 2020

Conversation

ang-zeyu
Copy link
Contributor

@ang-zeyu ang-zeyu commented Jun 28, 2020

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [x] Other, please explain: move sub packages to packages/ directory

Implements #1253 (comment)
Fixes #1060

What is the rationale for this request?
Standardise location of sub packages

What changes did you make? (Give an overview)

  • Move markbind core package to packages/markbind (preserved instead of core as we reference "the markbind core package..." quite a few times in docs)

  • Move frontend/components package to packages/vue-components

  • Use scoped package naming @markbind/core & @markbind/vue-components

  • Changed the respective path references in linting configurations, requires, package.json and the dev guide for the above

Proposed commit message: (wrap lines at 72 characters)
Move sub packages into /packages/ directory

The markbind core package resides in /src/lib/markbind/ directory of
the markbind-cli package, while the vue components package resides in
frontend/components.

Let's move these into /packages/ for consistent placing of
markbind-cli's dependencies.

Let's also rename the packages as scoped packages to reflect the new
directory structure.

docs/devGuide/projectManagement.md Outdated Show resolved Hide resolved
packages/core/src/Parser.js Outdated Show resolved Hide resolved
@ang-zeyu ang-zeyu force-pushed the refactor-package-directories branch 3 times, most recently from bdc4a59 to f5ed7fe Compare June 29, 2020 15:07
ang-zeyu added 4 commits June 30, 2020 21:17
@markbind/core's uses markbind-cli's logging utilities, which implies
it cannot function on its own.

The default logger instance as provided by winston and configured in
@markbind/core also includes the DailyRotateFile transport.
This causes unit tests for @markbind/core files that also mock the node
fs module to fail, since it is a file-based transport.

Let's restructure the logging slightly, providing @markbind/core with
it's own winston configuration without the DailyRotateFile transport.
Let's also increase the logging level of the console transport to
'debug', which was previously handled by the DailyRotateFile transport.
This ensures unit tests function properly, and allows @markbind/core to
function on its own.

Then, let's override this configuration in markbind-cli's
configuration, such that the old behaviour from a user standpoint is
preserved.
@ang-zeyu ang-zeyu force-pushed the refactor-package-directories branch from b5d3eaa to 9d89140 Compare June 30, 2020 13:17
@ang-zeyu
Copy link
Contributor Author

Thanks for the review @acjh!

Fyi:
Turns out the failing netlify build was due to the directory reorganization causing issues with netlify's node_modules cache. Makes sense looking back since none of the frontend/components dependencies were changed here

Added npm run ci:all && npm run build:components as well
@damithc we can now preview /play with any changes to components directly in the netlify build instead of screenshots / gifs 😬

@ang-zeyu ang-zeyu added this to the v2.15.0 milestone Jun 30, 2020
@ang-zeyu ang-zeyu merged commit 4fd662c into MarkBind:master Jun 30, 2020
@damithc
Copy link
Contributor

damithc commented Jun 30, 2020

@damithc we can now preview /play with any changes to components directly in the netlify build instead of screenshots / gifs 😬

Nice. Good work @ang-zeyu 💯

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.

Importing logger from componentParser.js cause parser.test.js to fail
3 participants