Skip to content

Conversation

@mikesir87
Copy link
Member

Proposed changes

After playing around with some docs changes, I was getting tired of having to rebuild and restart my containers every time I made a change. So, I decided to take a look at how to fix this. After all, it is a Jekyll project so we should be able to use jekyll serve, right?

  • Update our custom plugins to not produce file changes if nothing is being changed. Otherwise, we get in a constant file update loop
    • For the fetch_remote plugin, I added a completion_marker config option that serves as a filepath to determine if the fetch has already occurred. This prevents resource download from occurring on every file change.
  • Add a new dev stage to the Dockerfile that starts jekyll serve. It uses a new dev-start.sh script to still support the production test use case (which adds the production config file)
  • Update the docker-compose.yaml file to target this new dev stage and expose the livereload port

Related issues (optional)

None

The intent is to support jekyll serve without redownloading all
packages every time a change is made. With this, if the marker
file exists, no secondary download will occur.
Now, changes will be seen much faster, removing the requirement to
rebuild and restart the container every time you want to see a
change
@netlify
Copy link

netlify bot commented Jul 27, 2022

Deploy Preview for docsdocker ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit b66c509
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/62e1995f7b6ffe00084e1812
😎 Deploy Preview https://deploy-preview-15239--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@mikesir87
Copy link
Member Author

Saw the build failed and it's because of the new build/bake docs... all of the same files exist locally and in the remote repo to support the frontmatter. So, there's no way to really know if the files have already been fetched or not by simply using a marker file. Would be nice to have some mechanism to know we've fetched/setup the files without having to do so yet again.

@mikesir87
Copy link
Member Author

Same thing for the https://github.com/moby/buildkit repo too. The file it's loading already exists with the fetch_remote frontmatter.

@mikesir87
Copy link
Member Author

Ok. I think it's good to go for a review now. Bring on the feedback! 😄

@usha-mandya
Copy link
Member

Thank you @mikesir87.

@crazy-max @thaJeztah Could you PTAL?

@crazy-max
Copy link
Member

crazy-max commented Jul 30, 2022

@mikesir87 There are some conflicts, can you rebase please?

  • For the fetch_remote plugin, I added a completion_marker config option that serves as a filepath to determine if the fetch has already occurred. This prevents resource download from occurring on every file change.

We discussed about having better cache for fetch_remote plugin with @thaJeztah. Your completion_marker option might be good if current ref still has these files remotely otherwise it will break. It would also skip files newly added remotely.

As we currently download remote resources as zip and some references are not tied to a specific commit (e.g., using a branch like master) it's kinda hard to have proper caching. We should instead properly clone the repo and check for changes using git metadata.

Maybe if your use case is just live reloading local content we might instead just skip fetching remote resources as there is no hook of this type that can be used in Jekyll?

  • Add a new dev stage to the Dockerfile that starts jekyll serve. It uses a new dev-start.sh script to still support the production test use case (which adds the production config file)

Live reload lgtm but I think we should create another PR for this one and another to improve caching. WDYT?

@docker-robott
Copy link
Collaborator

Thanks for the pull request. We'd like to make our product docs better, but haven’t been able to review all the suggestions.
As our docs have also diverged, we do not have the bandwidth to review and rebase old pull requests.

If the updates are still relevant, review our contribution guidelines and rebase your pull request against the latest version of the docs, then mark it as fresh with a /remove-lifecycle stale comment.
If not, this pull request will be closed in 30 days. This helps our maintainers focus on the active pull requests.

Prevent pull requests from auto-closing with a /lifecycle frozen comment.

/lifecycle stale

@usha-mandya
Copy link
Member

@mikesir87 @crazy-max As we are gearing up to migrate Docs to Hugo, I'm wondering if we should bring in any changes from this PR to 'main' before the migration. WDYT?

@mikesir87
Copy link
Member Author

I can take another pass once the big PR is merged in. It won't break my heart to redo it 😆

@mikesir87 mikesir87 closed this Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants