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

update README ahead of UI one-click zero-config install #50

Merged
merged 1 commit into from
Nov 18, 2020

Conversation

lindsaylevine
Copy link

Fixes #44

this will definitely be a work in progress over the next day. please comment/suggest whatever.

@lindsaylevine lindsaylevine added the type: chore work needed to keep the product and development running smoothly label Nov 18, 2020
Copy link

@rstavchansky rstavchansky left a comment

Choose a reason for hiding this comment

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

@lindsaylevine I added a bunch of minor editing suggestions. Feel free to accept or not.
Looks good!

README.md Outdated
package = "@netlify/plugin-nextjs"
```

This plugin can also be installed and managed from your site's settings on Netlify.

Choose a reason for hiding this comment

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

I'm curious from @verythorough's POV whether the UI workflow is recommended over the TOML file-based workflow. If so, you may want to move this up to be the first installation option.

You can add something like this to link to the docs:

Read more about installing a plugin with the Netlify UI in our docs.

Choose a reason for hiding this comment

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

Yeah, I lean towards pitching the UI install first. We could start the installation section to say that the plugin can run with no configuration, and then present a link, probably in a text block or something to give it more attention, e.g.

👉 Click to install this plugin 👈

I went a little goofy, but you get the idea. 😛 (Incidentally, I'm now going to file an issue for an "Install this plugin" button.)

For the link, after https://github.com/netlify/netlify-react-ui/pull/6326 is merged, this link will work:

http://app.netlify.com/plugins/@netlify/plugin-nextjs/install

Until then, you can see what the experience will be like by testing this link (which is what I used in my emoji-laden example:

https://deploy-preview-6326--app.netlify.app/plugins/@netlify/plugin-nextjs/install

Copy link
Author

Choose a reason for hiding this comment

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

ok, i am going to handle the click to install link as a fast follow, but made a change suggesting ui install first pointing to the ui-based installation part of the docs !

This plugin can also be installed and managed from your site's settings on Netlify.
If you want to install there, you can omit this [[plugins]] section from your .toml file.

## Custom Netlify Functions
Copy link

Choose a reason for hiding this comment

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

Do we even need to mention custom Netlify Functions at all?

I am assuming if a site is using a Functions directory, they will assume this plugin to just work with it.
If a site is not using a Functions directory, then the default Functions directory will be used instead, which the user will not see.

Copy link
Author

Choose a reason for hiding this comment

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

tabling for after <3


[[plugins]]
package = "@netlify/plugin-nextjs"
```

[Read more about Netlify Functions here](https://docs.netlify.com/functions/overview/).

## Publish Directory
Copy link

Choose a reason for hiding this comment

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

Same comment as for Functions directory: do we even need to mention it?

Copy link

@ehmicky ehmicky left a comment

Choose a reason for hiding this comment

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

Thanks a lot @lindsaylevine!

Copy link

@verythorough verythorough left a comment

Choose a reason for hiding this comment

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

Looking good! I'm posting my review now so you can see the comments so far. Might have another one or two.

Comment on lines +31 to +40
[build]
command = "npm run build"

Choose a reason for hiding this comment

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

Is the build command necessary?

Copy link
Author

Choose a reason for hiding this comment

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

tabling for after <3

README.md Outdated
package = "@netlify/plugin-nextjs"
```

This plugin can also be installed and managed from your site's settings on Netlify.

Choose a reason for hiding this comment

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

Yeah, I lean towards pitching the UI install first. We could start the installation section to say that the plugin can run with no configuration, and then present a link, probably in a text block or something to give it more attention, e.g.

👉 Click to install this plugin 👈

I went a little goofy, but you get the idea. 😛 (Incidentally, I'm now going to file an issue for an "Install this plugin" button.)

For the link, after https://github.com/netlify/netlify-react-ui/pull/6326 is merged, this link will work:

http://app.netlify.com/plugins/@netlify/plugin-nextjs/install

Until then, you can see what the experience will be like by testing this link (which is what I used in my emoji-laden example:

https://deploy-preview-6326--app.netlify.app/plugins/@netlify/plugin-nextjs/install

@lindsaylevine lindsaylevine force-pushed the ll/config-updates-readme branch from ab154a1 to a5f8c6c Compare November 18, 2020 20:14
@lindsaylevine lindsaylevine force-pushed the ll/config-updates-readme branch from a5f8c6c to 247c0f2 Compare November 18, 2020 20:25
@lindsaylevine lindsaylevine merged commit 1b1013a into main Nov 18, 2020
@lindsaylevine lindsaylevine deleted the ll/config-updates-readme branch November 18, 2020 20:25
serhalp pushed a commit that referenced this pull request Apr 5, 2024
* feat: proxying res obj

* check blob for cachetag

* add cache-tags to pages/app

* added types

* removed hard coded deploy ID

* removed extra comma

* fix tag format for purge and add manifest

* refactoring

* add pageData var

* updated setcacheheaders logic

* initial testing for cache-tags

* updated unit test page data

* cache-tags integration tests for app

* page router tests

* integration test for pages revalidate

* lock file

* chore: fix tests

* chore: update tests

* feat: readd missing mockRequestHandlers

* chore: fix tests

---------

Co-authored-by: Rob Stanford <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore work needed to keep the product and development running smoothly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove configuration steps in README
4 participants