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

chore/script to module #960

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

carlosallexandre
Copy link
Contributor

Motivation

It turns the following scripts into modules: generate, log-build, watcher and hydrate.

This way, we execute the modules inside the respectively cli commands instead of executing them with node defined into package.json. Doing that brings us the possibility to measure the time spent with each function and post to the analytics in future improvements.

Copy link

changeset-bot bot commented Nov 15, 2024

⚠️ No Changeset found

Latest commit: 05d9f17

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@boyney123
Copy link
Collaborator

Thank you @carlosallexandre , will take a look.

Any reason default-files-for-collections has been moved here? Maybe we can do that stuff in seperate PR (if required), or does this PR require those changes?

@carlosallexandre
Copy link
Contributor Author

Any reason default-files-for-collections has been moved here? Maybe we can do that stuff in seperate PR (if required), or does this PR require those changes?

This makes the path to it into scripts/catalog-to-astro-content-directory.js more simple.

Now, the tsup configuration build to bin/dist/<files> having as entrypoint the bin/eventcatalog.ts. As we not specify the scripts/catalog-to-astro-content-directory.js in the entrypoint it will be in the bin/dist/eventcatalog.?(c)js. So, from there the default-files-for-collections is

const __filename = fileURLToPath(import.meta.url);
const rootPkg = path.resolve(
  path.dirname(__filename),
  process.env.NODE_ENV === 'test' ? '../' : '../../'
);
const defaultCollectionFilesDir = path.join(rootPkg, 'default-files-for-collections');

otherwise, if we keep the default-files-for-collections/ as is, it would be

  • structure
.
├── ...
├── bin                    
│   ├── dist
│   │   ├── eventcatalog.cjs
├── scripts                    
│   ├── default-files-for-collections
└── ...
  • path in scripts/catalog-to-astro-content-directory.js
const __filename = fileURLToPath(import.meta.url); // `bin/dist/eventcatalog.?(c)js`
const scriptsDir = path.resolve(
  path.dirname(__filename),
  process.env.NODE_ENV === 'test' ? '.' : '../../scripts/'
);
const defaultCollectionFilesDir = path.join(scripts, 'default-files-for-collections');

I chose not specify the scripts/catalog-to-astro-content-directory.js as an entrypoint because it would result into the following

.
├── ...
├── bin                    
│   ├── dist
│   │   ├── bin
│   │   │   ├── eventcatalog.cjs
│   │   ├── scripts
│   │   │   ├── catalog-to-content-astro-directory.js
├── scripts                    
│   ├── default-files-for-collections
└── ...

What I see with futures PR is

  • development structure
.
├── ...
├── astro # astro related files                     
├── default-files-for-collections 
├── src # the scripts dir today
├── eventcatalog.js # the entrypoint to cli
└── ... # other config files
  • the package in production would be
.
├── astro # astro related files                     
├── default-files-for-collections
├── dist # the src build
├── eventcatalog.js # the entrypoint to cli
└── ... # other needed files

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.

3 participants