-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
updated to 1.20.0 #5277
updated to 1.20.0 #5277
Conversation
Thank you for opening a PR to our samples repo and helping us upgrade our project 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ValerasNarbutas Awesome work 👏👏👏 You Rock 🤩
I managed to build it locally and played with it a bit.
During my review I noticed some small details we could sort out before we merge 👍. Please do give them a double check 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this file should not be here. The sample.json
should be present in the assets
folder in the root of the project and is used to store metadata for sample galleries. We should remove this instance here and leave only the one we already have in the react-birthday/assets
Along the way there we should update the SPFX-VERSION
to 1.20.0
@@ -1,58 +1,47 @@ | |||
{ | |||
"name": "birthdays-anniversary", | |||
"version": "2.0.0", | |||
"name": "happy-birthday", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why change the name?
"name": "birthdays-anniversary", | ||
"version": "2.0.0", | ||
"name": "happy-birthday", | ||
"version": "0.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should either not change the version number to make it higher but not smaller
@ValerasNarbutas do you mind taking a look at the code or instructions. The build seems to produce the following error in certain cases: |
Will do @hugoabernier |
thanks for checking @Adam-it and @hugoabernier , will review this. |
Hey @hugoabernier , did you indexed "Birthday", it is in instructions |
Many thanks @ValerasNarbutas for your update! Awesome!. Thank you for sharing your sample with others - you rock! 👏🥇👩💻 |
What's in this Pull Request?
SPFX upgrade to 1.20.0 version
Node Version
Node version used: 18.17.1
Checklist
README.md
file's Version history. For new samples, created a newREADME.md
file matching this templateREADME.md
has at least one static high-resolution screenshot (i.e. not a GIF) located in theassets
folder.README.md
contains complete setup instructions, including pre-requisites and permissions required.nvmrc
file indicating the version of Node.js