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

Updated namespace with keyInfo item; added 'anyOption' to records #383

Conversation

hudaif747
Copy link
Collaborator

Description

Related Issues

Design Decisions

Performance & Quality

Checklist

I, the author of this PR checked the following requirements for good software quality:

  • The code is properly formatted (I ran the formatter)
  • The code is written with our software quality standards (I ran the linter)
  • The code is written using our code style
  • Extensive in source documentation has been added
  • Unit and/or integration tests have been added
  • All texts have been internationalized with at least the following languages:
    • English
    • German
  • I tried addressing all new accessibility problems displayed in the console and documented if they can't be fixed
  • I attached performance measurements to prevent performance degradation
  • I added the changes to the next release section of the changelog

I, the reviewer checked the following things:

  • I ran the software once and tried all new and related functionality to this PR
  • I looked at all new and changed lines of code and commented on possible problems
  • I read the added documentation and checked if it is understandable and clear
  • I checked the added tests for completeness
  • I checked the internationalized strings for spelling errors
  • I checked the performance metrics for problems or unexplained degradation
  • I checked that the changes are noted in the changelog

Copy link
Collaborator

@NXXR NXXR left a comment

Choose a reason for hiding this comment

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

👍
the Prettier Job failed

check if running npm run format makes things better or worse.
If worse you can either remove the leading comments in the maps since I assume they are the reason why the formatter is unhappy. Otherwise, you can probably also exclude this file from the formatter somehow.

frontend/src/types/pandemos.ts Outdated Show resolved Hide resolved
frontend/src/types/pandemos.ts Outdated Show resolved Hide resolved
frontend/src/types/pandemos.ts Outdated Show resolved Hide resolved
frontend/src/types/pandemos.ts Outdated Show resolved Hide resolved
@NXXR
Copy link
Collaborator

NXXR commented Nov 12, 2024

I also checked the differences between feature/pandemos-datasocket-preprocessing and feature/pandemos there are barely any changes.
The files were moved and the KeyInfo-Stuff was moved from the datasocket to the types file.

Other than that the only major change is that the raw data is served as an array not a crossfilter anymore, and therefore an additional large crossfilter object with the agent and location infos merged into the trips is provided for @Pawandeep-Kaur-Betz statistics and @apoorvakay might also use this object to filter and display the map (if she doesn't use the raw trip and location array and filter based on the PandemosFilterSlice in the store)

You can check the datasocket code if there are any comments or todos that need fixing but I think most of it was optional for now.
If all looks good you can also merge the feature/pandemos-datasocket-preprocessing branch into the feature/pandemos branch.
make sure to let the others know as they probably need to pull the changes into their branches and modify their code accordingly.

Copy link

github-actions bot commented Nov 12, 2024

Test Results

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit 888ed18. ± Comparison against base commit de002a0.

♻️ This comment has been updated with latest results.

@NXXR
Copy link
Collaborator

NXXR commented Nov 12, 2024

Did you find the issue with the formatter @hudaif747 ?

@hudaif747
Copy link
Collaborator Author

Did you find the issue with the formatter @hudaif747 ?

The formatter fixed single qoutes in the pandemos.ts it did not mess with the rest of the file. The lint fix suggested to add disable comment for namespace and also to remove non-null assertions in the PandemosContext.tsx

@hudaif747 hudaif747 merged commit c99eea5 into feature/pandemos-datasocket-preprocessing Nov 12, 2024
2 of 5 checks passed
@hudaif747 hudaif747 deleted the feature/pandemos-namespace-update branch November 12, 2024 14:58
@hudaif747 hudaif747 restored the feature/pandemos-namespace-update branch November 13, 2024 09:42
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