Skip to content

Conversation

@felipecrs
Copy link
Contributor

@felipecrs felipecrs commented Jun 23, 2021

Motivation

My use case:

website/ # a repo
|   external/
|   |  repo-a/ # another repo, ignored by git at website/
|   |  repo-b/ # another repo, also ignored by git

With the current implementation, the lastUpdate won't return any data, as the files are unknown by Git when using website as the cwd. But if we call git in their own folders, git will be able to properly read the metadata.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

I verified by replacing the content of lastUpdate.js in my node_modules folder, which worked. But if needed, I can find a way to record a video. I just didn't record yet because in my tests there are private data from my company.

The good news is that this change won't break any existing use case, it will just fix mine.

Related PRs

None

@felipecrs felipecrs requested review from lex111 and slorber as code owners June 23, 2021 18:03
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 23, 2021
@netlify
Copy link

netlify bot commented Jun 23, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 08f8e98

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/60d37c8fba8573000804c2d2

😎 Browse the preview: https://deploy-preview-5048--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Jun 23, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 55
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-5048--docusaurus-2.netlify.app/

@slorber
Copy link
Collaborator

slorber commented Jun 24, 2021

Thanks, that looks fine 👍 as long as it does not break current lastUpdate for simpler setups

@slorber slorber merged commit f478262 into facebook:master Jun 24, 2021
@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Jun 24, 2021
@felipecrs felipecrs deleted the fix-last-update branch June 24, 2021 13:08
@slorber
Copy link
Collaborator

slorber commented Oct 24, 2025

Hi @felipecrs

Just wondering, is your site with multiple Git submodules opoen-source?

I'd be interested to check it because we'd like to optimize those git log calls as part of e18e/ecosystem-issues#216 and your setup might be a good representative example to benchmark against

@felipecrs
Copy link
Contributor Author

Unfortunately it is not. It would otherwise be a good benchmark candidate indeed!

(unless you want me to benchmark for you)

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

Labels

CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants