Skip to content

presentCountByDate and absentCountByDate #130

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

tejasai2007
Copy link

These queries calculate a member's present/absent days between two dates (excluding holidays), simplifying attendance tracking.

@ivinjabraham
Copy link
Member

ivinjabraham commented Jun 21, 2025

Hey @tejasai2007, thanks for the PR. I wanted to know if someone had asked you to make these queries in Root i.e. if this is part of a larger project involving Home.

Also, a few things about your PR and commits:

  • PR title should ideally be what the PR is doing in plain english.
  • Follow conventional commits for your commit messages. The "present and absent count" could have been something like "add: query for retrieving a member's attendance between two days". That's just a rough draft though.
  • Break down your commits logically. For example, in this current PR with one commit, you've added a dependency, changed formatting in main.rs, added a query and env. checking (which is redundant but more on that in the review). Every logical change should ideally be one commit.

These are some pretty standard practices in open source and it's very important that you respect them when contributing.

@@ -24,3 +24,4 @@ tracing = "0.1.41"
tracing-subscriber = { version = "0.3.19", features = ["env-filter", "time", "fmt", "std"] }
dotenv = "0.15.0"
time = { version = "0.3.37", features = ["formatting"] }
anyhow = "1.0.98"
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't seem to be used anywhere?

@@ -41,4 +41,6 @@ impl AttendanceQueries {

Ok(records)
}

Copy link
Member

Choose a reason for hiding this comment

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

Make sure you run clippy/linting tools to avoid unnecessary additions like this.

@@ -39,6 +42,11 @@ impl Config {

#[tokio::main]
async fn main() {
dotenv().ok();
Copy link
Member

Choose a reason for hiding this comment

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

Why do you think this is necessary?

@ivinjabraham
Copy link
Member

Also, next time use the fixes/closes keyword in the PR body to link it with it's issue. (#124)

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