-
Notifications
You must be signed in to change notification settings - Fork 0
feat: consolidate theme, update deps, add description field. #139
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
…ded-llc/ampdresume into feat/integrate-theme
feat: reintegrate theme into the main app.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Caution
Changes requested ❌
Reviewed everything up to 2eb5d21 in 3 minutes and 47 seconds. Click for details.
- Reviewed
11129lines of code in301files - Skipped
3files when reviewing. - Skipped posting
13draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/theme/default/components/pdf/Education.tsx:9
- Draft comment:
Using .map for side effects; consider using .forEach instead. - Reason this comment was not posted:
Marked as duplicate.
2. src/theme/default/components/Summary.tsx:54
- Draft comment:
When truncating long summaries, consider appending an ellipsis ('...') to indicate content is omitted. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. prisma/migrations/20250815004328_support_company_description/migration.sql:1
- Draft comment:
Typographical note: The comment "-- AlterTable" may be intended to read "-- Alter Table" (with a space) for clarity. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and suggests a typographical change for clarity. It doesn't provide a specific code suggestion or address a potential issue with the code's functionality. It doesn't align with the rules for useful comments.
4. src/app/edit/components/RichTextEditor/plugins/ColorPlugin.tsx:9
- Draft comment:
Typo: It looks like 'LOW_PRIORIRTY' might be a misspelling. Consider renaming it to 'LOW_PRIORITY' if that's what was intended. - Reason this comment was not posted:
Comment was on unchanged code.
5. src/app/edit/components/RichTextEditor/plugins/ToolbarPlugin.tsx:29
- Draft comment:
Typographical error: It looks like 'LOW_PRIORIRTY' might be meant to be 'LOW_PRIORITY'. Please correct the spelling. - Reason this comment was not posted:
Comment was on unchanged code.
6. src/app/edit/components/RichTextEditor/plugins/ToolbarPlugin.tsx:127
- Draft comment:
Typographical error: The parameterLOW_PRIORIRTYappears to be misspelled. It should likely beLOW_PRIORITY. - Reason this comment was not posted:
Comment was on unchanged code.
7. src/app/edit/components/RichTextEditor/plugins/YouTubePlugin.tsx:9
- Draft comment:
Typographical note: The function name is spelled as 'YoutubePlugin' in the code, but the file name and other references (e.g. YouTubeIcon) use 'YouTube'. Consider updating the function name to 'YouTubePlugin' for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
8. src/app/edit/components/RichTextEditor/plugins/YouTubePlugin.tsx:59
- Draft comment:
Typographical consistency: The placeholder text uses "Add Youtube URL" (with a lowercase 't') whereas the rest of the component uses "YouTube" (with an uppercase 'T'). Consider updating the placeholder to "Add YouTube URL". - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. src/app/edit/components/RichTextEditor/useKeyBindings.tsx:4
- Draft comment:
Typographical error: 'LOW_PRIORIRTY' appears to be misspelled. Did you mean 'LOW_PRIORITY'? - Reason this comment was not posted:
Comment was on unchanged code.
10. src/theme/components/Skills/SkillsExperience.tsx:43
- Draft comment:
Typo/lexicon issue: The comment on lines 40-42 refers to a property named 'yearStart', but the code uses 'yearStarted'. Consider harmonizing the naming to improve clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a new file being added. The mismatch between documentation and code could be confusing for future developers. However, the comment is about documentation alignment rather than a functional issue. The code itself works correctly. Documentation improvements are nice but not critical enough to block a PR. The mismatch could indicate a deeper issue where the data structure isn't well defined. Maybe the example is outdated and showing an old version of the API? While that's possible, we don't have enough context about the API. The code is clearly using 'yearStarted' successfully, so the example is likely just outdated. This comment should be deleted. While it points out a real documentation inconsistency, it's not critical enough to warrant a PR comment. The code itself is functional.
11. src/theme/default/components/WorkExperience/Projects.tsx:6
- Draft comment:
Typo: Consider changing "renders as list of all projects" to "renders a list of all projects" for improved clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment is technically correct about the grammar, it's an extremely minor documentation issue that doesn't affect code functionality or understanding. Documentation nitpicks like this create noise in PR reviews and don't meaningfully improve code quality. The meaning is perfectly clear even with the minor grammatical error. The grammar error could potentially confuse non-native English speakers. Documentation quality is part of code quality. While documentation quality matters, this error is so minor that it doesn't impact understanding, and fixing it doesn't meaningfully improve the codebase. PR comments should focus on more substantial issues. Delete this comment as it's too minor and doesn't meet the bar for PR comments. We should focus on more substantial issues.
12. src/theme/default/components/pdf/Skills.test.tsx:28
- Draft comment:
The comment says "Check for React's experience" but the test is actually verifying the 'React Redux' skill. Consider updating the comment to "Check for React Redux's experience". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While technically correct, this is an extremely minor documentation issue. The test is still clear and functional. The rules state not to make purely informative comments or obvious/unimportant changes. This feels like a nitpick that doesn't materially improve code quality or prevent bugs. The comment is factually correct and does point out an inconsistency. Poor test documentation could lead to confusion for future developers. While accurate, the inconsistency is minor and the test's intention remains clear from the surrounding context and code. This type of minor documentation fix creates noise without adding significant value. Delete this comment as it violates the rule about not making purely informative comments and addresses an unimportant documentation issue.
13. src/util/structure.ts:38
- Draft comment:
Typographical issue: The comment refers to a property 'yearStart', but the code accesses 'yearStarted'. Please ensure the property name is consistent with the documentation. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The inconsistency could be confusing for future developers. However, the example in the comment is just illustrative and doesn't need to match exactly. The actual property name yearStarted is more grammatically correct. The code itself works fine regardless of this documentation mismatch. This feels like a nitpick that doesn't require a code change. The inconsistency could genuinely confuse developers who are trying to understand the expected input format. Documentation should be accurate. While accuracy is important, this is just an illustrative example showing the general structure. The actual property name is clear from the code, and the example still effectively demonstrates the transformation concept. This comment should be deleted as it's too minor and doesn't require any actual code changes. The documentation example is clear enough despite the slight naming difference.
Workflow ID: wflow_bBSAUaTxuRoZIWaL
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thank you for submitting a PR! Please review & check the boxes below:
Description
This PR introduces a
descriptionfield on Companies to allow users to describe the company in brief.Additionally, dependencies are updated, and most importantly: the theme repository has been integrated directly into this repository for simplified maintenance and development.
Steps to Test
Spin up local,
npm run checkthennpm run buildandnpm cypress:runfor integration tests (all handled within this PR's GitHub Actions workflows anyway).descriptionfield support doesn't have an integration test yet, but I've tested locally and on thetestenv.Important
Consolidates theme repository, adds Company description field, updates dependencies, and enhances theme components with tests.
ThemeDavidsandThemeDefaultcomponents intheme/davids/ThemeDavids.tsxandtheme/default/ThemeDefault.tsx.descriptionfield toCompanymodel inprisma/schema.prismaandprisma/migrations/20250815004328_support_company_description/migration.sql.QRGeneratorcomponent intheme/davids/components/QRGenerator.tsxfor generating QR codes.SkillsSection,Summary, andWorkExperienceSectioncomponents intheme/davids/components/.MUIThemeProviderintheme/default/MUIThemeProvider.tsxfor theme management.SkillsExperience,CertificationsSection,FeaturedProjects, and more.package.jsonto remove@ampdresume/themedependency and addqrcode..eslintrc.json.This description was created by
for 2eb5d21. You can customize this summary. It will automatically update as commits are pushed.