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

added my builder profile #11

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Conversation

Superior212
Copy link
Contributor

@Superior212 Superior212 commented Oct 15, 2024

Description

  • Created my personal page under packages/nextjs/app/builders/0x62CeF3Ca8b52a9C69a17236CA2c56Cdb7a383E8e/page.tsx
  • Added bio, avatar, and links to social media
  • Verified the page structure and followed linting guidelines

Additional Information

address:0x62CeF3Ca8b52a9C69a17236CA2c56Cdb7a383E8e

image

Copy link

vercel bot commented Oct 15, 2024

@Superior212 is attempting to deploy a commit to the BuidlGuidl Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

vercel bot commented Oct 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
batch10-buidlguidl-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 21, 2024 0:21am

@Superior212
Copy link
Contributor Author

@derrekcoleman @phipsae updated the PR, is this more descriptive?

Copy link
Collaborator

@phipsae phipsae left a comment

Choose a reason for hiding this comment

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

Thank you very much for your personal page! It's simple, but looks great :)

Regarding the PR description please don't forget to relate to the respective issue (see https://gist.github.com/ZakGriffith/69d1eb8baebddd7d370b87a65a7e3ec0).

Please ensure that you don’t push changes to files unrelated to this PR. You can find my comments below.

packages/hardhat/contracts/BatchRegistry.sol Outdated Show resolved Hide resolved
packages/hardhat/contracts/CheckIn.sol Outdated Show resolved Hide resolved
packages/hardhat/deploy/00_deploy_your_contract.ts Outdated Show resolved Hide resolved
packages/hardhat/hardhat.config.ts Outdated Show resolved Hide resolved
packages/nextjs/.env.example Outdated Show resolved Hide resolved
packages/nextjs/contracts/deployedContracts.ts Outdated Show resolved Hide resolved
packages/nextjs/scaffold.config.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@phipsae phipsae left a comment

Choose a reason for hiding this comment

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

an additional comment below.

@Superior212
Copy link
Contributor Author

@derrekcoleman @phipsae fixed the issues Kindly assist me to check it out

Copy link
Collaborator

@derrekcoleman derrekcoleman left a comment

Choose a reason for hiding this comment

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

@Superior212, it's looking great! I'm following up on @phipsae's request to keep your PR focused and clean: each pull request should only do one thing, so a few files that were (accidentally?) included in your commits should be removed before we merge.

The main goal of the batch is to practice your collaboration and "git-fu" 🥷, so let us know if you would like any guidance using git rebase, git push --force, and other ways of manipulating the git history.

packages/hardhat/.env.example Outdated Show resolved Hide resolved
packages/hardhat/contracts/BatchRegistry.sol Outdated Show resolved Hide resolved
packages/nextjs/.env.example Outdated Show resolved Hide resolved
@Superior212
Copy link
Contributor Author

Superior212 commented Oct 16, 2024

@phipsae @derrekcoleman removed files

Copy link
Collaborator

@phipsae phipsae left a comment

Choose a reason for hiding this comment

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

In the beginning it can be challenging at first to contribute to repos, that's why we are having this course :)
As @derrekcoleman mentioned, it's crucial to keep the codebase intact and only add or modify the files necessary for your PR to implement the specific feature.

Please review the comments below and ensure that no code is deleted from the main branch, as we need this parts.

packages/hardhat/.env.example Outdated Show resolved Hide resolved
packages/hardhat/contracts/BatchRegistry.sol Outdated Show resolved Hide resolved
packages/nextjs/.env.example Outdated Show resolved Hide resolved
packages/nextjs/package.json Outdated Show resolved Hide resolved
packages/nextjs/package.json Show resolved Hide resolved
@Superior212
Copy link
Contributor Author

Superior212 commented Oct 17, 2024

@derrekcoleman @phipsae, please check it now, i have made the changes

Copy link
Collaborator

@phipsae phipsae left a comment

Choose a reason for hiding this comment

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

@Superior212 we are almost there!

See my comments below.

yarn.lock Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@phipsae phipsae left a comment

Choose a reason for hiding this comment

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

@Superior212 only one little change, and then you have it!

See my comment below and make sure that the yarn.lock file is also correct.

package.json Outdated
@@ -37,10 +37,15 @@
},
"packageManager": "[email protected]",
"devDependencies": {
"@types/react": "^18.3.5",
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are not needed here, as they belong and are in the nextjs folder package.json.

package.json Outdated
Comment on lines 46 to 49
},
"dependencies": {
"react": "^18.3.1",
"react-icons": "^5.3.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are not needed here, as they belong and are in the nextjs folder package.json.

@Superior212
Copy link
Contributor Author

@phipsae @derrekcoleman have removed the packages from the fille

@Superior212
Copy link
Contributor Author

@phipsae can I also start working on other issues immediately after this is merged?
I have updated it and it doesn't have any of my commit on the nextjs

@phipsae
Copy link
Collaborator

phipsae commented Oct 18, 2024

@Superior212 can you please run once more yarn install and push?

@Superior212
Copy link
Contributor Author

@phipsae done
thank you

@phipsae
Copy link
Collaborator

phipsae commented Oct 18, 2024

Finally we can merge!!! 🎉 Thank for your patience and page! :)

@Superior212
Copy link
Contributor Author

Thank you, so i can proceed with other issues

@phipsae phipsae merged commit 86346dc into BuidlGuidl:main Oct 18, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants