feat: move framework-info into build packages #4783

ericapisani · 2022-12-19T16:02:27Z

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes https://github.com/netlify/pod-ecosystem-frameworks/issues/330

Related issue: https://github.com/netlify/pillar-workflow/issues/1011

We want to eventually merge the functionality within framework-info into build-info since there's a great deal of overlap and having these 2 distinct projects is leading to a high maintenance burden.

This is the first step in this process, which will lead to https://github.com/netlify/framework-info/ being archived.

For us to review and ship your PR efficiently, please perform the following steps:

Open a bug/issue before writing your code 🧑‍💻. This ensures
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
Read the contribution guidelines 📖. This ensures your code follows our style guide and
passes our tests.
Update or add tests (if any source code was changed or added) 🧪
Update or add documentation (if features were changed or added) 📝
Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

github-actions · 2022-12-19T16:03:13Z

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

lukasholzer

Can you add the same typescript config as for the other libraries that allows js files as well and adapt the files to the dist folders?

packages/framework-info/.gitignore

packages/framework-info/CONTRIBUTING.md

packages/framework-info/CODE_OF_CONDUCT.md

lukasholzer · 2022-12-20T13:34:31Z

packages/framework-info/README.md

+[![npm version](https://img.shields.io/npm/v/@netlify/framework-info.svg)](https://npmjs.org/package/@netlify/framework-info)
+[![Coverage Status](https://codecov.io/gh/netlify/framework-info/branch/main/graph/badge.svg)](https://codecov.io/gh/netlify/framework-info)
+[![Build](https://github.com/netlify/framework-info/workflows/Build/badge.svg)](https://github.com/netlify/framework-info/actions)
+[![Downloads](https://img.shields.io/npm/dm/@netlify/framework-info.svg)](https://www.npmjs.com/package/@netlify/framework-info)


I think they need to be updated

Would you be open to having these just removed outright? Personally I'm not sure how useful they are as statuses here

let's remove them :)

packages/framework-info/package.json

needed to add 'root' key in order to support invoking the build command from the repository root

ericapisani · 2022-12-20T20:12:43Z

@lukasholzer

Can you add the same typescript config as for the other libraries that allows js files as well and adapt the files to the dist folders?

At the moment we can't as there are no Typescript files in the package, and if we attempt to add the configuration files now we'll get an error about there being no input files 😞

I'd suggest doing this as a follow up pull request where we convert 1 file to Typescript (even if it's not fully typed) and add the configuration in as part of that

(( edit: Ignore the above, it turned out that if I added .js files as part of the include property it removes the error ))

… ep/add-framework-info

This reverts commit 812d123.

This reverts commit 6696e3b.

… ep/add-framework-info

ericapisani · 2023-01-03T17:38:07Z

packages/framework-info/tsconfig.json

+{
+  "extends": "../../tsconfig.base.json",
+  "compilerOptions": {
+    "noEmit": true /* Specify an output folder for all emitted files. */


Vite handles transpilation of TS files by default so this is left to allow for type checking prior to building with vite

Addressed comments and got another code review confirming the changes are good

danez

We need to add it to the release-please configs:
https://github.com/netlify/build/blob/main/.release-please-manifest.json
https://github.com/netlify/build/blob/main/release-please-config.json

… ep/add-framework-info

Addressed comment in 48640be

danez · 2023-01-05T16:10:13Z

I think refactor: will not trigger a release-please release, so it should be feat: or fix: when merging, if you want a release

ericapisani · 2023-01-05T20:04:24Z

@danez Good point, I always forget about that

lukasholzer

small comments

packages/framework-info/CONTRIBUTING.md

packages/framework-info/README.md

… ep/add-framework-info

Co-authored-by: Lukas Holzer <lukas.holzer@netlify.com>

… ep/add-framework-info

depends on netlify/build#4783

* docs: add deprecation notice depends on netlify/build#4783 * style: lint

… ep/add-framework-info

Co-authored-by: Lukas Holzer <lukas.holzer@netlify.com>

refactor: move framework-info into build packages

a64548d

github-actions bot added the Adds or modifies js files label Dec 19, 2022

ericapisani assigned ericapisani and lukasholzer Dec 19, 2022

lukasholzer previously requested changes Dec 20, 2022

View reviewed changes

ericapisani added 3 commits December 20, 2022 14:29

refactor: address some of the code review comments

ed168b6

refactor: remove more things

0a8c4d9

refactor: update vite config

a08a1d7

needed to add 'root' key in order to support invoking the build command from the repository root

Merge branch 'main' into ep/add-framework-info

a662256

ericapisani marked this pull request as ready for review December 21, 2022 18:01

ericapisani requested a review from lukasholzer December 21, 2022 18:01

ericapisani added 5 commits December 21, 2022 13:06

chore: ignore test fixtures and output dir for linter

6696e3b

Merge branch 'main' into ep/add-framework-info

dadacf8

test: update test snapshots

812d123

Merge branch 'ep/add-framework-info' of github.com:netlify/build into…

e728c67

… ep/add-framework-info

Revert "test: update test snapshots"

cb74d72

This reverts commit 812d123.

ericapisani mentioned this pull request Dec 22, 2022

Investigate failing snapshot tests in @netlify/build package #4790

Closed

ericapisani added 4 commits December 22, 2022 08:50

Merge branch 'main' into ep/add-framework-info

1f32edd

Merge branch 'main' into ep/add-framework-info

51141c4

Revert "chore: ignore test fixtures and output dir for linter"

658d746

This reverts commit 6696e3b.

Merge branch 'ep/add-framework-info' of github.com:netlify/build into…

4edeadf

… ep/add-framework-info

ericapisani commented Jan 3, 2023

View reviewed changes

jobala previously approved these changes Jan 4, 2023

View reviewed changes

Merge branch 'main' into ep/add-framework-info

c2b10cb

danez previously requested changes Jan 4, 2023

View reviewed changes

ericapisani added 2 commits January 5, 2023 09:28

refactor: add framework-info to release-please config

48640be

Merge branch 'ep/add-framework-info' of github.com:netlify/build into…

382f5f4

… ep/add-framework-info

ericapisani dismissed jobala’s stale review via 382f5f4 January 5, 2023 14:29

ericapisani changed the title ~~refactor: move framework-info into build packages~~ feat: move framework-info into build packages Jan 5, 2023

Merge branch 'main' into ep/add-framework-info

d6ae147

ericapisani requested a review from danez January 6, 2023 16:34

Merge branch 'main' into ep/add-framework-info

7baabb6

lukasholzer reviewed Jan 9, 2023

View reviewed changes

packages/framework-info/CONTRIBUTING.md Outdated Show resolved Hide resolved

packages/framework-info/CONTRIBUTING.md Outdated Show resolved Hide resolved

packages/framework-info/README.md Outdated Show resolved Hide resolved

ericapisani and others added 4 commits January 9, 2023 10:49

docs: updates

982b093

Merge branch 'ep/add-framework-info' of github.com:netlify/build into…

ead481e

… ep/add-framework-info

Apply suggestions from code review

2948bfd

Co-authored-by: Lukas Holzer <lukas.holzer@netlify.com>

Merge branch 'ep/add-framework-info' of github.com:netlify/build into…

e157066

… ep/add-framework-info

ericapisani requested a review from lukasholzer January 9, 2023 18:33

ericapisani added a commit to netlify/framework-info that referenced this pull request Jan 9, 2023

docs: add deprecation notice

974960e

depends on netlify/build#4783

ericapisani mentioned this pull request Jan 9, 2023

docs: add deprecation notice netlify/framework-info#853

Merged

Merge branch 'main' into ep/add-framework-info

21d5c61

kodiakhq bot pushed a commit to netlify/framework-info that referenced this pull request Jan 10, 2023

docs: add deprecation notice (#853)

db4604f

* docs: add deprecation notice depends on netlify/build#4783 * style: lint

ericapisani added 2 commits January 10, 2023 08:54

style: liiiiiint

5f5dcd0

Merge branch 'ep/add-framework-info' of github.com:netlify/build into…

46fde62

… ep/add-framework-info

lukasholzer approved these changes Jan 10, 2023

View reviewed changes

ericapisani added the automerge label Jan 10, 2023

ericapisani merged commit d37cbc4 into main Jan 10, 2023

ericapisani deleted the ep/add-framework-info branch January 10, 2023 15:40

netlify-circleci mentioned this pull request Jan 10, 2023

chore: release main #4805

Merged

jobala pushed a commit that referenced this pull request Jan 11, 2023

feat: move framework-info into build packages (#4783)

c3c7635

Co-authored-by: Lukas Holzer <lukas.holzer@netlify.com>

feat: move framework-info into build packages #4783

feat: move framework-info into build packages #4783

Uh oh!

Conversation

ericapisani commented Dec 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

github-actions bot commented Dec 19, 2022

Uh oh!

lukasholzer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukasholzer Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

ericapisani Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

lukasholzer Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ericapisani commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericapisani Jan 3, 2023

Choose a reason for hiding this comment

Uh oh!

danez left a comment

Choose a reason for hiding this comment

Uh oh!

danez commented Jan 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericapisani commented Jan 5, 2023

Uh oh!

lukasholzer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ericapisani commented Dec 19, 2022 •

edited

Loading

ericapisani commented Dec 20, 2022 •

edited

Loading

danez commented Jan 5, 2023 •

edited

Loading