Skip to content
This repository has been archived by the owner on Dec 5, 2020. It is now read-only.

Unwind some aspects of monorepoization #164

Closed
wincent opened this issue Feb 27, 2019 · 1 comment
Closed

Unwind some aspects of monorepoization #164

wincent opened this issue Feb 27, 2019 · 1 comment
Milestone
9.0.0-alpha.0

Comments

@wincent
Copy link
Contributor

wincent commented Feb 27, 2019

As mentioned in #163 we have some near-identical duplication in this repo which stems from it being a monorepo. For example, two copies of "divert.js" because it is easier to copy-paste the code between two NPM packages than it is to extract the code into a common package and depend on that; every additional NPM package adds a little bit of overhead.

I asked about the historical reasons for this being a monorepo and it seems that they are mostly just that, and we should feel free to revise that decision as we see fit. It may be that the "liferay-theme-deps-7.x" packages still make sense to be independent, but there are certainly others that may as well get folded into a single "main" package, like "liferay-theme-finder" which probably doesn't have a strong reason to independently exist (if you really, really wanted to use it independently, there would be nothing to stop you requiring if from the "main" package; the unit of atomicity that developers interact with the SDK is really just "the SDK", and that means the Yeoman generator and the gulp tasks).

So, the idea of this issue is not to completely undo the monorepo structure in the repo, but see if there is some consolidation that we can do to make development more streamlined. With fewer interdependent packages, it should be easier to test, release and build, and we won't have accidental side-effects of the monorepo pattern like instances of copy-pasted code.
@wincent wincent added this to the 8.0.0-rc.4 milestone Feb 27, 2019
wincent added a commit that referenced this issue Feb 27, 2019
@wincent
Fold "divert" into liferay-theme-tasks
140e08f 
As noted in:

#163

This function was duplicated almost verbatim in two packages. Centralize
it in "liferay-theme-tasks" as the first step towards unmonorepoization
discussed in:

#164

Eventually, we might only have two packages:

- generator-liferay-theme (needs to be independent, so yeoman can find
  it).
- liferay-theme-tasks (effectively the public "API" of the SDK).

Test plan: `yarn test` and `yo ./packages/generator-liferay-theme`
(works, apart from expected failure due to us not having published
liferay-theme-deps-7.2 to NPM yet).
wincent added a commit that referenced this issue Feb 27, 2019
@wincent
Fold "divert" into liferay-theme-tasks
24c3c1d 
As noted in:

#163

This function was duplicated almost verbatim in two packages. Centralize
it in "liferay-theme-tasks" as the first step towards unmonorepoization
discussed in:

#164

Eventually, we might only have two packages:

- generator-liferay-theme (needs to be independent, so yeoman can find
  it).
- liferay-theme-tasks (effectively the public "API" of the SDK).

Test plan: `yarn test` and `yo ./packages/generator-liferay-theme`
(works, apart from expected failure due to us not having published
liferay-theme-deps-7.2 to NPM yet).
This was referenced Feb 27, 2019
Merged
Closed
wincent added a commit that referenced this issue Feb 27, 2019
@wincent
Remove deprecated liferay-theme-finder package
8ef723c 
We're wanting to reduce complexity here, for the reasons stated in:

#164

This package has 0 weekly downloads, and its functionality has already
been folded in to "liferay-theme-tasks":

https://github.com/liferay/liferay-themes-sdk/blob/84db0037fa76d8d6d405316a5790ac2e9f9111d9/packages/liferay-theme-tasks/lib/theme_finder.js

Closes: #166
This was referenced Feb 27, 2019
Merged
Closed
wincent added a commit that referenced this issue Feb 27, 2019
@wincent
Fold "divert" into liferay-theme-tasks
481470f 
As noted in:

#163

This function was duplicated almost verbatim in two packages. Centralize
it in "liferay-theme-tasks" as the first step towards unmonorepoization
discussed in:

#164

Eventually, we might only have two packages:

- generator-liferay-theme (needs to be independent, so yeoman can find
  it).
- liferay-theme-tasks (effectively the public "API" of the SDK).

Test plan: `yarn test` and `yo ./packages/generator-liferay-theme`
(works, apart from expected failure due to us not having published
liferay-theme-deps-7.2 to NPM yet).
wincent added a commit that referenced this issue Feb 27, 2019
@wincent
Inline liferay-theme-deps-normalize side effect into generator
fcf4b73 
Related: #164
wincent added a commit that referenced this issue Feb 28, 2019
@wincent
Inline liferay-theme-deps-normalize side effect into generator
82cc87b 
Related: #164
wincent added a commit that referenced this issue Mar 7, 2019
@wincent
Remove liferay-theme-deps-7.0 (#164, #184)
09a91d1 
No longer required in the v9 branch. Next steps after this will be to
see if we can remove liferay-theme-deps-7.1 and then switch from
liferay-theme-deps-7.2 into individual, explicit dependencies.

Related: #164
Related: #184
wincent added a commit that referenced this issue Mar 8, 2019
@wincent
Remove liferay-theme-deps-7.0 (#164, #184)
dc7e88b 
No longer required in the v9 branch. Next steps after this will be to
see if we can remove liferay-theme-deps-7.1 and then switch from
liferay-theme-deps-7.2 into individual, explicit dependencies.

Related: #164
Related: #184
@wincent wincent modified the milestones: 8.0.0-rc.4, 9.0.0 Mar 14, 2019
@wincent
Copy link
Contributor Author

wincent commented Mar 14, 2019

We have effectively done as much as we're going to do of this for the v9 release in #187 — principally removing the "liferay-theme-deps-*" packages. We can look at doing more later on, but it seems non-urgent at this point.

@wincent wincent closed this as completed Mar 14, 2019
wincent added a commit that referenced this issue Mar 15, 2019
@wincent
Remove liferay-theme-es2015-hook (toolkit v8) (#110, #164, #199)
ce6a145 
Three reasons to delete this:

1. It doesn't actually work:
   #110
2. We wanted to simplify the structure of the monorepo anyway:
   #164
3. It has ancient dependencies which add noise to `yarn audit`:
   #199

`yarn audit` goes from:

    105 vulnerabilities found - Packages audited: 139619
    Severity: 38 Low | 31 Moderate | 36 High

to:

    94 vulnerabilities found - Packages audited: 135401
    Severity: 34 Low | 28 Moderate | 32 High

with this change.

We target v8 of the toolbox in this change, but I will make the
equivalent change in the "master" (v9) branch as well.

A later step will be to address #110, but that will involve completely
rethinking and reimplenting how we do ES2015 support.
wincent added a commit that referenced this issue Mar 15, 2019
@wincent
Remove liferay-theme-es2015-hook (toolkit v9) (#110, #164, #199)
9037923 
This is the v9 ("master" branch) version of ce6a145.

Three reasons to delete this:

1. It doesn't actually work:
    #110
2. We wanted to simplify the structure of the monorepo anyway:
    #164
3. It has ancient dependencies which add noise to `yarn audit`:
    #199

`yarn audit` goes from:

    43 vulnerabilities found - Packages audited: 533368
    Severity: 16 Low | 11 Moderate | 16 High

to:

    32 vulnerabilities found - Packages audited: 529201
    Severity: 12 Low | 8 Moderate | 12 High

with this change.

A later step will be to address #110, but that will involve completely
rethinking and reimplenting how we do ES2015 support.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
9.0.0-alpha.0
Development

No branches or pull requests
1 participant
@wincent