269: Allow for directly accessing epic owners

[JensAstrup]

No description provided.

[github-actions]

🚫 [eslint] <yenz/type-ordering> _{reported by reviewdog 🐶}
Definition for rule 'yenz/type-ordering' was not found.

[github-actions]

🚫 [eslint] <no-loops/no-loops> _{reported by reviewdog 🐶}
Definition for rule 'no-loops/no-loops' was not found.

[github-actions]

🚫 [eslint] <yenz/type-ordering> _{reported by reviewdog 🐶}
Definition for rule 'yenz/type-ordering' was not found.

[github-actions]

🚫 [eslint] <no-loops/no-loops> _{reported by reviewdog 🐶}
Definition for rule 'no-loops/no-loops' was not found.

[github-actions]

Reviewing the code changes provided across different files:

.eslintrc.cjs

• Positive Feedback: Including baseConfig.parserOptions with the spread operator (...) is an efficient way to extend the base configuration while adding or overriding specific properties. This is a good use of existing configurations to maintain consistency across your projects.

package.json

• Positive Feedback: Updating the version to 4.1.1-alpha.0 appropriately follows semantic versioning principles, signaling a new feature addition in a way that is clear to other developers and systems.
• Adding the eslint-plugin-jest package shows attention to maintaining code quality and adherence to best practices in test files, which is commendable.

src/epics/epic.ts

• Import improvements and consistency in code formatting (spacing around braces) contribute to better readability and adherence to common JavaScript/TypeScript coding standards.
• The inclusion of the get owners() functionality is a great addition, expanding the class capabilities with more structured and accessible data.
• Correct usage of AxiosError and AxiosResponse types enhances error handling and response parsing, making the code more robust and maintainable.
• Utilizing generic type parameters in convertApiFields calls ensures type safety, making the code more predictable and easier to debug.

tests/epics/epic.test.ts

• Positive Feedback: The use of async/await with .resolves in tests is a clear and modern approach to writing asynchronous tests in Jest. This makes the tests more readable and easy to understand.
• Mocking external dependencies, such as axios, is correctly implemented, ensuring that the tests are focused on the functionality of Epic without relying on external services.
• Extending the tests to cover new getter methods like followers and owners shows diligence in maintaining comprehensive test coverage as new features are added.

TypeScript Configuration Files (tsconfig.*.json)

• The adjustments in the various tsconfig.*.json files, such as including tests in the compilation context and simplifying config inheritance, are good practices. Including tests alongside source files helps ensure that both are kept in sync in terms of TypeScript compilation options.
• Removing specific include configs from the derived tsconfig files (tsconfig.cjs.json and tsconfig.esm.json) in favor of depending on the base configuration reduces duplication and potential for inconsistency.

General Observations and Recommendations:

• There are no major concerns related to non-functioning code or significant best-practice deviations. The changes mostly reflect enhancements, refactoring for better structure, and improvements in coding standards.
• It's good to see adherence to best practices such as extending configurations and maintaining uniformity in code formatting.
• All changes appear to serve a specific purpose, either improving code maintainability, enhancing testing, or ensuring stricter type safety.

This code review did not uncover any critical issues requiring immediate action beyond the comments made. The changes are generally positive and reflect a thoughtful approach to improving the codebase.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.67%. Comparing base (9983449) to head (addebd4).
Report is 3 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #274      +/-   ##
===========================================
- Coverage    92.33%   90.67%   -1.66%     
===========================================
  Files           50       50              
  Lines         2479     2489      +10     
  Branches       175      167       -8     
===========================================
- Hits          2289     2257      -32     
- Misses         190      232      +42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

This review covers a range of files committing changes across configuration, source, and test code. Here are the critical observations, suggestions, and positive affirmations where appropriate:

.eslintrc.cjs File

1. The addition of spreading baseConfig.parserOptions and baseConfig.plugins is a solid approach to extending eslint configurations. This practice maintains a clean, modular, and reusable setup that is beneficial for managing project-wide linting rules effectively.

package.json File

1. Updating the version to 4.1.1-alpha.0 appropriately follows semantic versioning conventions for pre-release versions. This change indicates new features or fixes that are still in testing before the final release.
2. Adding eslint-plugin-jest as a dependency is a best practice for projects using Jest for testing. It aids in maintaining code quality and following best jest practices.

TypeScript Configuration Files

1. The inclusion of test files in the include array of tsconfig.base.json is a commendable addition for ensuring TypeScript compiles and checks types in test files as well.
2. Removal of the include key from tsconfig.cjs.json and tsconfig.esm.json relies on the base configuration, which is a good practice to avoid repetition and maintain consistency across configurations.

Source Code (src/epics/epic.ts)

1. Import optimization and formatting show attention to consistency and code quality. For instance, combining import lines from axios and adding spaces in object literals enhances readability and maintainability.
2. The introduction of the owners getter provides a clear, concise way to access an epic's owners, demonstrating a thorough understanding of JavaScript's asynchronous patterns and class property usage.

Test Code (tests/epics/epic.test.ts)

1. The transition to using async/await syntax in tests for objectives and teams is a significant improvement, making tests more straightforward and reflecting modern JavaScript practices.
2. Proper mocking of axios and service methods within tests demonstrates a strong grasp of Jest's testing capabilities, specifically around mocking external dependencies.
3. The additional tests for followers and owners getters are great for ensuring the reliability and robustness of new features.

General Comments

1. The overall structure and organization of the changes indicate a thoughtful approach to codebase evolution, especially in enhancing the eslint setup and improving TypeScript configuration.
2. The consistent use of modern JavaScript features, like async/await and spread syntax, aligns well with best practices and enhances code clarity and maintainability.
3. It's also worth noting the removal of duplicate entries (e.g., "@typescript-eslint" plugin mentioned twice in .eslintrc.cjs) which could have been an oversight but wasn't made. Always look out for such redundancies to keep configurations clean and efficient.

In conclusion, the changes made across different files demonstrate a thoughtful and strategic approach to improving the codebase's quality and maintainability. Good job on applying best practices and optimizing the project setup and structure for better development efficiency.