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.

Sign up for GitHub

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

Jump to bottom

housekeeping #4

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

housekeeping #4

wants to merge 10 commits into from

Conversation

i10416
Copy link

@i10416 i10416 commented Feb 27, 2022 •
edited
Loading

  • add github actions to run linter and test for PR
  • update analysis_options.yaml more pedantic
  • address linter warning
  • address errors from wrong type inferences for json string.
  • improve type safety
  • mark immutable classes with const

@i10416
small housekeeping
0af60c0 
- add github actions to run linter and test for PR
- update analysis_options.yaml more pedantic
- address linter warning
-
@i10416 i10416 changed the title small housekeeping wip: small housekeeping Feb 27, 2022
@i10416
Copy link
Author

i10416 commented Feb 27, 2022 •
edited
Loading

At the moment, tests failed for 25 22 specs. I'm working on these cases.

@i10416
Copy link
Author

i10416 commented Feb 27, 2022

https://github.com/i10416/mustache/runs/5352061825?check_suite_focus=true

@i10416
fix: avoid var for immutable values
0bc75ee
@i10416
fix: collection#removeLast can return null if empty
6eea5f6
@i10416
chore: exclude inheritance from test
0082634 
- inheritance feature is newer compered to other mustache features and
  should be implemented in dedicated PR.
@i10416
Copy link
Author

i10416 commented Feb 28, 2022 •
edited
Loading

At the moment, tests failed for 25 22 specs. I'm working on these cases.

22 tests are failed due to this library lacking inheritance feature.
Because inheritance feature is relatively new, I skip inheritance tests for now. I feel it makes much sense to implement the feature and tests in another dedicated PR.

@i10416
Copy link
Author

i10416 commented Feb 28, 2022

@jonahwilliams Hello and I appreciate you maintaining this library.

I made some housekeeping changes and fixed small bugs in json parsing and spec. Could you review changes when you have spare time?

I'm happy to tweak these changes, so please feel free to tell me if there's anything you care about. Thank you, in advance.

@i10416
Copy link
Author

i10416 commented Feb 28, 2022

Sorry, I found a lot of linter warning show up after updating linter config to be type-safer. I will add some changes.

@i10416 i10416 changed the title wip: small housekeeping small housekeeping Feb 28, 2022
@i10416 i10416 changed the title small housekeeping housekeeping Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@i10416