Skip to content
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

Typescript #4619

Draft
wants to merge 14 commits into
base: v3
Choose a base branch
from
Draft

Typescript #4619

wants to merge 14 commits into from

Conversation

joonaojapalo
Copy link
Contributor

Proposed Changes

Pull Request Check List

  • A reasonable set of unit tests is included
  • Console does not show new warnings/errors
  • Changes are documented or they are self explanatory
  • This pull request does not have any merge conflicts
  • All existing tests pass in CI build
  • Code coverage does not decrease (unless measured incorrectly)

Review

  • Read and verify the code changes
  • Test the functionality by running the UI locally with all popular browsers available in your platform
  • Check that the implementation matches the design, when such one is defined in a Jira issue
  • Merge the pull request

Copy link
Member

@optionsome optionsome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should enable tests for pull requests against this otp2 branch. However, I think we maybe should rename the otp2 branch to be v3 instead so it follows normal versioning (although it's not as clear what the changes are intended for in the branch) before we change the github actions to run against pull requests.

I noticed a couple of things, the .nycrc.json file should be updated so it takes typescript files into account. Also, I think the relay does not work for typescript files currently. I think the support for typescript has improved in the react-relay version 13. We could try updating the relay related dependencies instead of doing some extra configuration/adding dependencies that will no longer be needed later on. We should also look into what would be the most optimal way for us to use the graphql types for typescript (maybe it can be done directly through relay?). I'm not sure if the relay/graphql related changes should be done in this pr or in a separate pr as for files that don't use relay this basic setup seems to work.

@joonaojapalo joonaojapalo changed the title [WIP] Typescript Typescript Aug 24, 2022
@joonaojapalo
Copy link
Contributor Author

However, I think we maybe should rename the otp2 branch to be v3 instead so it follows normal versioning (although it's not as clear what the changes are intended for in the branch) before we change the github actions to run against pull requests.

@optionsome please see #4620. I already renamed the branch otp2 to v3 in Github UI (by accident), but didn't revert as this seems to be our desired state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants