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

feat: add ci for prettier check and build #48

Closed
wants to merge 63 commits into from

Conversation

kunal00000
Copy link
Contributor

@kunal00000 kunal00000 commented Nov 21, 2023

Added CI for prettier check and build on PRs and pushes.
Resolves #47

Comment on lines +20 to +21
- name: Run prettier
run: npm run prettier:check
Copy link
Collaborator

@marcelovicentegc marcelovicentegc Nov 22, 2023

Choose a reason for hiding this comment

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

Static tests could run on git hooks/pre-commits as well along with a linter 😄. What do you think?

@kunal00000
Copy link
Contributor Author

I gave rebasing a shot, and I'm crossing my fingers that I got it right. 🧐 I always feel a bit on edge when I'm rebasing branches, especially when there are conflicts involved.

steve8708 and others added 7 commits November 26, 2023 12:28
Refactor getPageHtml function to handle selector not found case, using body as fallback. Add support for downloading URLs from sitemap.xml. Update comments to let know that sitemap is supported
chore: add `!package-lock.json` and `!tsconfig.json` to `.gitignore`
Add .gitignore entries for pnpm-lock.yaml
@steve8708
Copy link
Contributor

apologies, we just merged a few more things, needs one more round of conflict resolution and then we can get this in!

Fix for Issue BuilderIO#66 | Docker run fails due to "Cannot find module '/home/myuser/dist/main.js'" | Error in Docker container
@kunal00000
Copy link
Contributor Author

@steve8708 resolved conflicts ✅

@marcelovicentegc
Copy link
Collaborator

@kunal00000, I see lots of overrides from changes that were already integrated to the main branch on this PR. It occurs to me that this might introduce some inconsistencies to the git history. Can you open a new PR including the prettier check? It will help us keep the repository and its git history clean 😄

@marcelovicentegc
Copy link
Collaborator

Also, here's a step by step to rebase and preserve the repo git history from a forked repo (adapted from https://stackoverflow.com/a/7244456/9317004)

# Add the remote, call it "upstream":

git remote add upstream git@github.com:BuilderIO/gpt-crawler.git

# Fetch all the branches of that remote into remote-tracking branches

git fetch upstream

# Make sure that you're on your main branch:

git checkout main

# Rewrite your master branch so that any commits of yours that
# aren't already in upstream/master are replayed on top of that
# other branch:

git rebase upstream/main 

Copy link
Collaborator

@marcelovicentegc marcelovicentegc left a comment

Choose a reason for hiding this comment

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

I'm leaving a suggestion to open a new PR including only the changes for the prettier check step on CI since this PR duplicates the git history (per #48 (comment))

@kunal00000
Copy link
Contributor Author

Thanks @marcelovicentegc 🤠. created another PR for this.
Closing this one

@kunal00000 kunal00000 closed this Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CI