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

Fix: Correct the typo in method name #4369

Closed
wants to merge 5 commits into from

Conversation

Sauter001
Copy link
Contributor

  • The typo of the name of the function getHightLightDaysMap was corrected. (getHightLightDaysMap -> getHighLightDaysMap )

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ This pull request was sent to the PullRequest network.


@Sauter001 you can click here to see the review status or cancel the code review job.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

PullRequest Breakdown

Reviewable lines of change

+ 43
- 10

62% Markdown
23% JavaScript (tests)
15% JavaScript

Type of change

Fix - These changes are likely to be fixing a bug or issue.

@Sauter001 Sauter001 changed the title Fix: Correct typos Fix: Correct the typo in method name Nov 10, 2023
Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

Thanks for making a new PR for the typo fix (and for fixing the typo!)

I assume you meant to remove the template change but haven't yet.

Image of Dallas Dallas


Reviewed with ❤️ by PullRequest

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

The typo fix itself looks good. To turn it into a non-breaking change, it may be useful to instead create a new function with the correct name, and then have getHightLightDaysMap call that and emit a deprecation warning. Also, it looks like the template from another PR snuck into this PR.

Image of Eric E Eric E


Reviewed with ❤️ by PullRequest

Copy link

codecov bot commented Nov 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.67%. Comparing base (0ca150d) to head (00b46b8).

❗ Current head 00b46b8 differs from pull request most recent head 75158eb. Consider uploading reports for the commit 75158eb to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4369      +/-   ##
==========================================
- Coverage   96.90%   96.67%   -0.23%     
==========================================
  Files          28       27       -1     
  Lines        2581     2374     -207     
  Branches     1094      966     -128     
==========================================
- Hits         2501     2295     -206     
+ Misses         80       79       -1     

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

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

The typo fix itself looks good. To turn it into a non-breaking change, it may be useful to instead create a new function with the correct name, and then have getHightLightDaysMap call that and emit a deprecation warning. Also, it looks like the template from another PR snuck into this PR.

Oh, great suggestion about the deprecation warning.

Image of Dallas Dallas

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✖️ This code review was cancelled. See Details

@martijnrusschen
Copy link
Member

Are you expecting to follow up on this pull request?

@@ -8,21 +8,64 @@ assignees: ""

## Description

**Linked issue**: #(issue number)
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

This is unrelated

@mirus-ua
Copy link
Contributor

Hello

This one will be major, so you can update your PR to include it in the release

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

Successfully merging this pull request may close these issues.

None yet

3 participants