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: update the readme #29

Merged
merged 2 commits into from
Oct 23, 2024
Merged

chore: update the readme #29

merged 2 commits into from
Oct 23, 2024

Conversation

irbull
Copy link
Contributor

@irbull irbull commented Oct 23, 2024

Update the README.md file to reflect the current state of the project. This includes adding a section on how to build and contribute to the project and updates the note about who is currently maintaining the project.

This change-set also removes the CHANGELOG.md file as most Deno projects don't include this and it was mostly a change log of the Astro project.

Update the README.md file to reflect the current state of the project.
This includes adding a section on how to build and contribute to the
project and updates the note about who is currently maintaining the
project.

This change-set also removes the CHANGELOG.md file as most Deno projects
don't include this and it was mostly a change log of the Astro project.
@CLAassistant
Copy link

CLAassistant commented Oct 23, 2024

CLA assistant check
All committers have signed the CLA.

You can also check our [Astro Integration Documentation][astro-integration] for
more on integrations.
To configure your development environment, clone the repository and install
[`pnpm`](https://pnpm.io/). `pnpm` is a package manager that emphasizes disk
Copy link
Member

Choose a reason for hiding this comment

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

Do you prefer to use pnpm? I'm personally in favor of getting rid of pnpm settings (Currently the repo has both pnpm-lock.yaml and package-lock.json and that feels confusing to me)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't prefer pnpm, but when I tried npm i the build / test didn't work. I just tried it again, and it seems you get errors related to "File not found". I assumed that we rely on the pnpm disk layout vs the one that npm uses.

Also pnpm is the tool used in the GitHub CI, so this keeps the developers setup closer to that of GitHub. Maybe we can take a pass at removing pnpm and using npm (or even moving to Deno 2.0 and using Deno to provision the dependencies).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is also a deno.lock file ;). I guess we don't need the package-lock.json? Should we remove that one, assuming that we really do need pnpm to build this project right now?

Copy link
Member

Choose a reason for hiding this comment

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

Ok. Let's revisit that in later PRs

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

@kt3k kt3k merged commit aaa1125 into denoland:main Oct 23, 2024
2 checks passed
@und-miller
Copy link

NPM link to the changelog should be removed: https://www.npmjs.com/package/@deno/astro-adapter#changelog

@und-miller und-miller mentioned this pull request Oct 30, 2024
@irbull
Copy link
Contributor Author

irbull commented Oct 30, 2024

NPM link to the changelog should be removed: https://www.npmjs.com/package/@deno/astro-adapter#changelog

Good catch. Actually that entire page is out-of-date. I've updated the README, but it's not reflected there. I wonder who has access to update the NPM description.

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.

4 participants