-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
fix: add validation step to check if the event date is in the future. #751
fix: add validation step to check if the event date is in the future. #751
Conversation
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
# New step to validate if the event is in the future | ||
- name: Validate if the event is in the future | ||
uses: actions/github-script@v6 | ||
with: | ||
script: | | ||
const eventDate = new Date('${{ inputs.date }}T${{ inputs.time }}:00Z'); | ||
const currentDate = new Date(); | ||
if (eventDate <= currentDate) { | ||
core.setFailed('The event cannot be scheduled in the past.'); | ||
} |
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.
# New step to validate if the event is in the future | |
- name: Validate if the event is in the future | |
uses: actions/github-script@v6 | |
with: | |
script: | | |
const eventDate = new Date('${{ inputs.date }}T${{ inputs.time }}:00Z'); | |
const currentDate = new Date(); | |
if (eventDate <= currentDate) { | |
core.setFailed('The event cannot be scheduled in the past.'); | |
} | |
// validate if the event is in the future | |
const eventDate = new Date('${{ inputs.date }}T${{ inputs.time }}:00Z'); | |
const currentDate = new Date(); | |
if (eventDate <= currentDate) { | |
core.setFailed('The event cannot be scheduled in the past.'); | |
} |
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 can add this script to the previous step :)
@KhudaDad414 Thank you for your review. I have made the changes. 🙂 |
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.
LGTM! thanks @princerajpoot20 🙏
@princerajpoot20 did you test this locally? can you show proof that you did? |
@AceTheCreator I have tried testing it, but faced some difficulty in setting up locally. Sorry for making a PR without testing it locally. |
@princerajpoot20 it's hard to test actions locally. I would recommend testing workflows in your fork. it's much easier. |
Not a problem mate 🙂 |
Thanks for validating it 🙂 |
its working this how you wanted to be ?
|
@KhudaDad414 Ok, I will try testing in my fork and thanks for validating.🙂 |
@fmvilas it would be a quick review. 🙇 |
if (eventDate <= currentDate) { | ||
core.setFailed('The event cannot be scheduled in the past.'); |
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.
please make it more detailed, I mean the error message:
- specify what was the event date specified by the user
- specify what is the current time
when user makes a mistake, they make it unintentionally so definitely good to show them in logs what went wrong
@derberg I agree with your point 🙂. I have made the changes. |
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.
LGTM, well done 👏🏼 thanks!!!
@KhudaDad414 since you reviewed it before, please have another look and merge when ready |
/rtm |
thanks @princerajpoot20 🙇 |
@allcontributorsbot please add @princerajpoot20 for code |
@allcontributors please add @princerajpoot20 for code |
I've put up a pull request to add @princerajpoot20! 🎉 |
@derberg @KhudaDad414 Thank you 🙇 |
Description