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

fix: nested packages detection #294

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

emmenko
Copy link
Contributor

@emmenko emmenko commented Apr 14, 2020

Until now the library assumes that packages are located under packages/* and tries to derive the package name from the file path (see packageFromPath method).

This is fragile and it does not work anyway if packages are located in other directories or sub-directories.

This PR changes how package names are detected by implementing the getPackages method in the @lerna/project package.
I decided to copy over the important parts for a couple of reasons:

  • the lerna packages are not typed
  • avoid an unnecessary dependency to the lerna packages

Now the packages are properly detected, based on the glob configuration in the yarn workspaces or lerna.json.

Comment on lines +79 to +84
const fullFilePath = path.join(this.config.rootPath, filePath);
const matchingPackageFromPath = workspacePackages.find(packageInfo => {
return fullFilePath.startsWith(packageInfo.location);
});
if (matchingPackageFromPath) return matchingPackageFromPath.name;
return "";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the new logic to get the package name.

@@ -20,9 +21,7 @@ export interface ConfigLoaderOptions {
}

export function load(options: ConfigLoaderOptions = {}): Configuration {
let cwd = process.cwd();
let rootPath = execa.sync("git", ["rev-parse", "--show-toplevel"], { cwd }).stdout;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really related to this PR, but I figured the git command belongs to the git file.

@@ -5,7 +5,7 @@ exports[`createMarkdown multiple tags outputs correct changelog 1`] = `
## a-new-hope@4.0.0 (1977-05-25)

#### :rocket: New Feature
* \`a-new-hope\`
* \`@star-wars/a-new-hope\`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mock data contains scoped package names, hence these changes.

Comment on lines +19 to +33
a0000003: ["packages/star-wars/a-new-hope/rebels.js"],
a0000004: ["packages/star-wars/a-new-hope/package.json"],
a0000005: ["packages/star-wars/empire-strikes-back/death-star.js"],
a0000006: ["packages/star-wars/empire-strikes-back/death-star.js"],
a0000007: ["packages/star-wars/empire-strikes-back/hoth.js"],
a0000008: ["packages/star-wars/empire-strikes-back/hoth.js"],
a0000009: ["packages/star-wars/empire-strikes-back/package.json"],
a0000010: ["packages/star-wars/return-of-the-jedi/jabba-the-hutt.js"],
a0000011: ["packages/star-wars/return-of-the-jedi/vader-luke.js"],
a0000012: ["packages/star-wars/return-of-the-jedi/leia.js"],
a0000013: ["packages/star-wars/return-of-the-jedi/package.json"],
a0000014: [
"packages/star-wars/the-force-awakens/mission.js",
"packages/star-wars/origin-stories/rogue-one/mission.js",
],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to ensure that the "nested" package locations work.

@@ -106,45 +110,75 @@ const issuesCache = {
},
},
};

describe("multiple tags", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved inside the describe("createMarkdown", for consistency

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

Successfully merging this pull request may close these issues.

None yet

1 participant