Netlify integration #30

denar90 · 2020-02-16T00:55:09Z

Add support for integration with Netlify Preview Site deploy

User changes stuff for the site and creates PR -> Netlify deploy Preview site -> Action do the check against Preview site and show send notification via github/slack

Example I tested stuff

repo - https://github.com/test-denar/one-click-hugo-cms-3
action - https://github.com/test-denar/one-click-hugo-cms-3/pull/3/checks?check_run_id=448353220
Netlify stuff - https://app.netlify.com/sites/practical-allen-be16f3/deploys

Todo:

docs
race conditions, Netlify can deploy longer then action starts to check, we need to wait for the Netlify to finish and then run action
add/improve options

P.S. I'll squash commits after final round of feedback :)

denar90 · 2020-02-23T21:44:41Z

Yay! Ping works
Test workflow - https://github.com/test-denar/one-click-hugo-cms-3/runs/463513341?check_suite_focus=true

denar90 · 2020-02-29T21:37:08Z

@exterkamp can I ask you to take a look? 👀

exterkamp

These are cool changes, overall they seem good. I have some questions though.

Can I provide urls that go to a single netlify site, or multiple sites? Can I mix netlify sites and regular sites?

Why do we need a netlifySite param? Seems like we could look at the domains of the URLs and tell that they need to be adjusted. However, then how do you mix preview-deploy URLs and normal netlify hosted URLs? Is this something we want to support.

I'm also curious if this integrates with the netlify github action? Is there a way to show how you would use both in the recipe?

README.md

src/index.js

exterkamp · 2020-02-29T22:54:01Z

src/index.js

  core.startGroup('Action config')
  console.log('Input args:', input)
  core.endGroup() // Action config

+  /*******************************WARMING UP***********************************/


I wonder if this should be called "Warming up" or something like "Waiting for preconditions"?

exterkamp · 2020-02-29T22:54:32Z

src/index.js

  core.startGroup('Action config')
  console.log('Input args:', input)
  core.endGroup() // Action config

+  /*******************************WARMING UP***********************************/
+  if (input.netlifySite) {
+    core.startGroup('Warming up')


Maybe this would be more descriptive with a group like "Waiting for Netlify site"

src/index.js

exterkamp · 2020-03-01T00:04:11Z

src/index.js

+        }
+        console.log('Waiting for build to be done')
+        await sleep()
+        console.log(`Retry ping Netlify`)


These retry messages seem excessive, since there will be a "Pinging..." log as soon as the function is called.

exterkamp · 2020-03-01T00:06:24Z

src/index.js

+          console.log('Resolve Netlify site error', e)
+          return Promise.resolve(1)
+        }
+        console.log('Waiting for build to be done')


Seems like these waiting msg's could be DRY'd up?

I've updated, hope I got your point :)

src/index.js

exterkamp · 2020-03-01T00:11:34Z

src/index.js

+    const retryNumber = 5
+    let runs = 0
+    const sleep = async () => {
+      return new Promise(r => setTimeout(r, 60000))


Maybe a good idea to pull out the 60000 ms out so that it can be modified easier/used to dynamically output how long the system is waiting in console logs.

Then maybe modify the logs to say Waiting additional ${seconds} seconds for Netlify build to be done....

I've been thinking about it for the next iteration after getting feedback, I didn't wanna overwhelm action settings :)

Co-Authored-By: Shane Exterkamp <[email protected]>

denar90 · 2020-03-01T22:33:18Z

Can I provide urls that go to a single netlify site, or multiple sites? Can I mix netlify sites and regular sites?

What's the use case, using Netlify urls, and some other urls in scope of one repository? My intention was the next, if user has deployed netlify site from github, then in scope this repo he/she can use action for that urls.

Why do we need a netlifySite param?

Sorry that's my bad, I'll update doc. So, Netlify site can have custom domain, and in this case we can't compose preview url properly. Thank you for bringing it up, It's a really good point because I haven't tested this one yet 👍

However, then how do you mix preview-deploy URLs and normal netlify hosted URLs? Is this something we want to support.

I think it's good to have support of real site and preview. Preview is composed based on PR info -

lighthouse-ci-action/src/input.js

Lines 130 to 136 in 5935285

    
           if (ref) { 
        
             origin = `https://${ref}--${netlifySite}` 
        
           } 
        
           if (isNumber(Number.parseInt(ref))) { 
        
             origin = `https://deploy-preview-${ref}--${netlifySite}` 
        
           }

If it's a master, aka action config for push event, url will be like https://master--practical-allen-be16f3.netlify.com/, but if it's PR, url will look like https://deploy-preview-1--practical-allen-be16f3.netlify.com, where number is a number of PR in repo.

I'm also curious if this integrates with the netlify github action? Is there a way to show how you would use both in the recipe?

Cool, need to play with it. Will get back with info :)

Co-Authored-By: Shane Exterkamp <[email protected]>

denar90 · 2020-03-01T23:04:57Z

@exterkamp thank you so much for the review and questions 😍 I hope to find time to during the week to add more comments to code and also to clarify all statements related to the confusion between different urls to run. Feel free to ask more :)

alekseykulikov · 2020-03-19T19:23:57Z

@denar90 and I agreed, instead of a custom integration, provide an official Netlify recipe, using one of the community actions: https://github.com/marketplace?type=actions&query=netlify

denar90 changed the base branch from master to slack-integration February 16, 2020 00:55

denar90 mentioned this pull request Feb 21, 2020

[Previews] Netlify integration #31

Closed

denar90 added 15 commits February 29, 2020 11:30

Use Netlify site

61c0224

try github context

e4d2ff8

Get branch name

f068202

map urls path for preview

fc1353b

Better preview url

b0c1fb8

Handle PR ref

1562e41

Handle PR ref

cdd210d

Handle PR ref

bd8f1e4

Handle PR ref

fc7d5f4

Handle PR ref

eea7091

Handle PR ref

1434813

Handle PR ref

80f20a8

Try ping

28017a3

add node-fetch

9d184cf

Improve linting, docs

d200ba3

denar90 force-pushed the netlify branch from 715db33 to d200ba3 Compare February 29, 2020 21:14

denar90 added 3 commits February 29, 2020 23:15

Improve linting

f8c699d

Add note about process of Nelify preview availability

1bdb6a8

Remove Netlify workflow

d404b76

denar90 marked this pull request as ready for review February 29, 2020 21:27

denar90 changed the base branch from slack-integration to next February 29, 2020 21:28

Improve linting

317d98f

exterkamp requested changes Mar 1, 2020

View reviewed changes

denar90 and others added 3 commits March 1, 2020 23:00

Update src/index.js

79f0b96

Co-Authored-By: Shane Exterkamp <[email protected]>

Update README.md

bda2372

Co-Authored-By: Shane Exterkamp <[email protected]>

Update README.md

a44148a

Co-Authored-By: Shane Exterkamp <[email protected]>

denar90 and others added 2 commits March 1, 2020 23:01

Update src/index.js

0d2a8a6

Co-Authored-By: Shane Exterkamp <[email protected]>

Update src/index.js

5935285

Co-Authored-By: Shane Exterkamp <[email protected]>

denar90 and others added 3 commits March 2, 2020 00:39

Cleanup

f8107cc

More cleanup

27c8c92

Update src/index.js

139d7f3

Co-Authored-By: Shane Exterkamp <[email protected]>

This was referenced Mar 6, 2020

Next fixes, iteration #2 #35

Merged

Next (Review, Feedback) #36

Closed

alekseykulikov force-pushed the next branch 3 times, most recently from ddc438f to 5954e70 Compare March 12, 2020 09:56

alekseykulikov closed this Mar 19, 2020

alekseykulikov deleted the netlify branch March 19, 2020 19:22

alekseykulikov mentioned this pull request Mar 19, 2020

Add more recipes and use-cases #20

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Netlify integration #30

Netlify integration #30

denar90 commented Feb 16, 2020 •

edited

Loading

denar90 commented Feb 23, 2020

denar90 commented Feb 29, 2020

exterkamp left a comment

exterkamp Feb 29, 2020

exterkamp Feb 29, 2020

exterkamp Mar 1, 2020

exterkamp Mar 1, 2020

denar90 Mar 1, 2020 •

edited

Loading

exterkamp Mar 1, 2020

denar90 Mar 1, 2020

denar90 commented Mar 1, 2020

denar90 commented Mar 1, 2020

alekseykulikov commented Mar 19, 2020

Netlify integration #30

Netlify integration #30

Conversation

denar90 commented Feb 16, 2020 • edited Loading

Add support for integration with Netlify Preview Site deploy

denar90 commented Feb 23, 2020

denar90 commented Feb 29, 2020

exterkamp left a comment

Choose a reason for hiding this comment

exterkamp Feb 29, 2020

Choose a reason for hiding this comment

exterkamp Feb 29, 2020

Choose a reason for hiding this comment

exterkamp Mar 1, 2020

Choose a reason for hiding this comment

exterkamp Mar 1, 2020

Choose a reason for hiding this comment

denar90 Mar 1, 2020 • edited Loading

Choose a reason for hiding this comment

exterkamp Mar 1, 2020

Choose a reason for hiding this comment

denar90 Mar 1, 2020

Choose a reason for hiding this comment

denar90 commented Mar 1, 2020

denar90 commented Mar 1, 2020

alekseykulikov commented Mar 19, 2020

denar90 commented Feb 16, 2020 •

edited

Loading

denar90 Mar 1, 2020 •

edited

Loading