Skip to content

feat: remove PRISMA_FMT_BINARY references - #4292

Merged
ruheni merged 4 commits into
mainfrom
feat/remove-prisma-fmt
Feb 7, 2023
Merged

feat: remove PRISMA_FMT_BINARY references#4292
ruheni merged 4 commits into
mainfrom
feat/remove-prisma-fmt

Conversation

@jkomyno

@jkomyno jkomyno commented Dec 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

DO NOT MERGE UNTIL prisma@4.10.0 is ready to be released.

Describe this PR

Follow-up for prisma/orm#16923.
We've stopped using the prisma-fmt formatter binary in prisma@4.3.0. Although we're currently still downloading the binary, we're no longer using it. As a result, PRISMA_FMT_BINARY has been a useless environment variable for months.

Changes

Remove references to PRISMA_FMT_BINARY.

What issue does this fix?

Contributes to prisma/orm#16924

@github-actions

github-actions Bot commented Dec 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR changes the following pages (Vercel Preview Deploy links):

Changed Vercel links

(Note that links will only be valid after Vercel preview deploy succeeded)

@jkomyno jkomyno added this to the 4.9.0 milestone Dec 20, 2022
@vercel

vercel Bot commented Dec 20, 2022 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 7, 2023 at 7:07PM (UTC)

@jkomyno

jkomyno commented Dec 20, 2022

Copy link
Copy Markdown
Contributor Author

Note: @Jolg42 and I had a doubt about specifying the behavior of PRISMA_FMT_BINARY with an asterisk pointing out that it's no longer in use as of prisma@4.3.0. However, we figured that custom Prisma binaries is such an edge case scenario that it's probably not worth it to keep mentions of PRISMA_FMT_BINARY around.

@jkomyno jkomyno modified the milestones: 4.9.0, 4.10.0 Jan 12, 2023
@jkomyno
jkomyno marked this pull request as ready for review February 7, 2023 09:09
@jkomyno
jkomyno requested a review from ruheni February 7, 2023 09:09
@janpio

janpio commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

Where did the documentation of the env var move for users of older Prisma versions?
We only have one docs website, so we need to make sure it also works for other versions.
In this case we could for example add a "In older versions" headline that has a minimal mention of what the env var used to to and be with info when we removed it.

@janpio janpio mentioned this pull request Feb 7, 2023
Merged
@janpio

janpio commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

The other PR https://github.com/prisma/docs/pull/4495/files for INtrospection Engine also updates some prisma -v output and serverless framework instructions from the docs which seems to have been skipped here.

@ruheni
ruheni merged commit 5601ae0 into main Feb 7, 2023
@ruheni
ruheni deleted the feat/remove-prisma-fmt branch February 7, 2023 19:08
janpio
janpio reviewed Feb 7, 2023
View reviewed changes
Comment thread content/400-reference/200-api-reference/300-environment-variables-reference.mdx
- The Introspection Engine is served by the Migration Engine from [4.9.0](https://github.com/prisma/prisma/releases/tag/4.9.0). Therefore, the `PRISMA_INTROSPECTION_ENGINE` environment variable will not be used.

</Admonition>
### CLI Binary Targets

@janpio janpio Feb 7, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why move this headline up here? PRISMA_FMT_BINARY has nothing to do with "CLI Binary Targets".

@ruheni ruheni Feb 8, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I must have made a mistake when applying the feedback. I'll make the change in another PR.

@ruheni ruheni Feb 8, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be covered by #4502

janpio
janpio reviewed Feb 7, 2023
View reviewed changes
Comment thread content/400-reference/200-api-reference/300-environment-variables-reference.mdx
PRISMA_INTROSPECTION_ENGINE_BINARY=custom/my-introspection-engine-unix
```

<Admonition type="warning">

@janpio janpio Feb 7, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why remove this unrelated admonition about a different env var? The fmt change does not invalidate this.

@ruheni ruheni Feb 8, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#4502 fixes this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@janpio janpio janpio left review comments
@ruheni ruheni ruheni approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.10.0

Development

Successfully merging this pull request may close these issues.

3 participants

@jkomyno @janpio @ruheni