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

refactor: Replace Status factory functions with static objects #3093

Merged

Conversation

ilandikov
Copy link
Collaborator

@ilandikov ilandikov commented Sep 23, 2024

Types of changes

Internal changes:

  • Refactor (prefix: refactor - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
  • Tests (prefix: test - additions and improvements to unit tests and the smoke tests)

Description

  • Status.make*() are same as the existing Status.* calls (* is any status), so
    • Replace Status.make*() status methods with existing Status.* static class fields
    • For other statuses, refactor Status.make*() method calls to Status.* static class fields

For reference: existing Status.* static class fields were calling Status.make*() methods, so the behaviour was same.

Note from @claremacrae: the behaviour differed in how many Status instances were created, however.

Motivation and Context

  • remove code duplication and use shorter calls to Status static methods
  • better code

How has this been tested?

  • unit tests

Checklist

  • My code follows the code style of this project and passes yarn run lint.
  • My change has adequate Unit Test coverage.

Terms

src/Statuses/Status.ts Outdated Show resolved Hide resolved
src/Statuses/Status.ts Outdated Show resolved Hide resolved
Copy link

@claremacrae
Copy link
Collaborator

Looking at how little code has changed in src, and most of the changes being in tests, it may be that my slight doubts are irrelevant!

@claremacrae claremacrae changed the title refactor: status makers refactor: Replace Status factory functions with static objects Sep 24, 2024
@claremacrae claremacrae added type: internal Only regards development or contributing scope: task states Requests relating to custom task states, like 'in-progress', for example, and state transitions labels Sep 24, 2024
@claremacrae claremacrae merged commit 914e858 into obsidian-tasks-group:main Sep 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: task states Requests relating to custom task states, like 'in-progress', for example, and state transitions type: internal Only regards development or contributing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants