-
Notifications
You must be signed in to change notification settings - Fork 0
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
Superhero-Search-Project #3
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.
Hi @bhushan354,
Good job so far 👍🏻
There are some issues that you still need to work on to go to the next project but you are almost there!
STATUS: REQUIRE CHANGES 🔴
Highlights
- Good PR title ✅
- Descriptive PR description ✅
- Implement the features ✅
Required Changes ♻️
- Because your file changes are not reflected in this PR, that's why I am gonna add some comments here. I appreciate your hard work, you did an amazing job by adding your creativity, and design logic and I am happy that instead of 2 you made
three
pages. But you should stick with project requirements, by strongly following the design wireframe, according to which, you should have 2 column design only, and you need to make sure that your application should use only one colour theme, not multiple colours so that's why you need to make some updates to meet the project requirement, your second page I agreed you have two columns but your first page should also have the same design as second. Kudos for thesearch
functionality which is working fine.
- As I mentioned about colour, you can choose the colour of your own choice but your layout should also use the same colour except for the colours for the text and links. I can see that you
are using more than 2 colours, so please adhere to the requirements and follow the wireframe colour properly.
- Kindly take a look and compare your design, your design looks different in terms of styling, please fix your design styles because adhering to the project requirement will help you to be a better developer and detail-oriented
Design Comparison
Behance Design | Your Design |
---|---|
- Please make sure that your application is warm or error-free, sometime Warnings are not that important for us while development, but if you are getting a warning for something really important then you should fix it because as you can see in the below screenshot, you got a warning of missing
attribute
Key for the element running inside the loop but becauseReact
uses the virtual dom and always make the required changes to the real DOM and behind the scenes, this type of keys help the react to differentiate between the things and maintain the uniqueness to perform the render or repaint the component based on our action.
- As I mentioned already about your design, I would also take your attention to your detail page, you should make your detail page as per the wireframe provided by Microverse, it seems like you put in your efforts but neglected the important requirement, but I still believe you can do make the changes to pass the requirements.
- Always try to separate your test files from your components, you test file should be inside the
__test__
folder just likecomponent
folders which makes your code base clean and easy to follow.
Check the comments under the review.
Optional suggestions
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you take them into account as they can make your code better.
Cheers, and Happy coding!👏👏👏
⚠️ WARNING ⚠️
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
Have any doubt ❓
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag @skyv26 in your question so I can receive the notification. You can also follow me on the below platforms
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
README.md
Outdated
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 add the
video
presentation link as per the requirement, your video should not be more than 5 min. You can use the loom for recording and then make a public shareable link. So that we as CR can look into that in order to get a review of what you did and what is special about your project. Kindly stick with the requirements tightly.
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.
- Well resolved
README.md
Outdated
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.
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.
- [REQUIRED] Kindly add a professional project description to this project. The previous reviewer only recommended that you add the relevant description to the README not that you should remove it completely. A professional README needs a standard project description.
|
||
## 🚀 Live Demo <a name="live-demo"></a> | ||
|
||
- [[Live Demo link](https://bhushan354.github.io/Superhero-Search-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.
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.
- [REQUIRED] This project is still yet to be deployed as recommended by the previous reviewer and as stated by the project requirement. Kindly deploy your project properly and share the correct deployed link in the README file. Kindly deploy your project properly and add the deployment link here.
|
||
Contributions, issues, and feature requests are welcome! | ||
|
||
Feel free to check the [issues page](https://github.com/bhushan354//issues) | ||
|
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.
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.
- well resolved
## 🙏 Acknowledgments <a name="acknowledgements"></a> | ||
|
||
I would like to thank the Microverse team and Code reviewers who helped us create this project. Also, I would like to express our sincere gratitude to [Agustine Soria](https://github.com/SaveryIV) for his valuable assistance in solving a problem with fetching rockets From API. His expertise and guidance Ire instrumental in finding a solution. |
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.
- Well done, I really appreciate your sincerity in acknowledging your peer/fellow/friend who helped you with the project but as per the requirement, you should acknowledge first the designer of the web application from whom you took the inspiration to design the application. Please follow the requirements carefully and update this section with acknowledgement for the designer of the metrics apps.
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.
- well resolved
Hello sir , I hope these changes fullfills the criteria but if there are some issues , it would be of great help to me if you tell me on whatsapp +918600118932 , as I have time constraint . Thank you for consideration, |
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.
Hi @bhushan354
Good job so far!
There are some issues that you still need to work on to go to the next project but you are almost there!
Required Changes ♻️
- Check the inline comments under the review for sections to improve.
Optional suggestions
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
N/A
Cheers and Happy coding!
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me @Whoistolu in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
|
||
|
||
## 📝 License <a name="license"></a> | ||
|
||
This project is [MIT](./MIT) licensed. | ||
This project is [MIT](./LICENSE) licensed. |
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.
- Well resolved
@@ -96,7 +94,7 @@ To get a local copy up and running follow these simple example steps. | |||
|
|||
Contributions, issues, and feature requests are welcome! | |||
|
|||
Feel free to check the [issues page](https://github.com/bhushan354//issues) | |||
Feel free to check the [issues page](https://github.com/bhushan354/Superhero-Search-Project/issues) |
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.
- Well resolved
hello sir you have written well resolved in here so what does it mean @Whoistolu |
You are almost there @bhushan354 🥇 . Kindly check the comments I added for the remaining changes under the comments of the previous reviewer. The project description on the README and the deployment are still pending. Let's fix those 👍🏽 |
sir I dont know why the github pages is deploying the README @Whoistolu , can you help me with that , please!! |
So sorry about this experience @bhushan354 , kindly ensure that you have selected the correct branch to deploy from. Pease ensure that you are selecting the |
yes sir I have selected it correctly @Whoistolu |
Hello @bhushan354 could you check your slack? |
Hello sir/mam , I hope these changes fullfills the criteria but if there are some issues , it would be of great help to me if you tell me on whatsapp +918600118932 , as I have time constraint and have to submit this project in less than 1 hour and my everything else is completed . Thank you for consideration, |
yes wait |
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.
Changes Required ♻️
Hi @,
Nice job, you are almost there.
(Highlights) Good Points: 👍
- Your work is well documented. ✔
- The correct workflow is used. ✔
- All linters checks are ok. ✔
(Changes Required) Aspects to improve: ♻️
- Please check the required changes here:
As required by the last reviewer, Please add a proper project description here to the readme file.
[Optional] suggestions:
- Nothing to mention.
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear, and please remember to tag me in your question so I can receive the notification**
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
In This PR I have used Following things while implementing features in my webApp :
Thank you ,
Bhuhsan .