-
Notifications
You must be signed in to change notification settings - Fork 9
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
build(deps-dev): bump credo from 1.7.1 to 1.7.2 #2339
Conversation
Coverage of commit
|
lib/schedule/piece.ex
Outdated
@spec for_now?(t(), Util.Time.time_of_day()) :: boolean() | ||
def for_now?(piece, now_time_of_day) do | ||
piece.start_time <= now_time_of_day and piece.end_time > now_time_of_day |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion+nit(non-blocking): what if we renamed this to active?
or active_at_time?
?
Or if we wanted to match how block_waiver.ex
does it, current?
?
I'd do this in a refactor PR, but I'm curious what y'all think.
cc: @mbta/skate-developers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spec for_now?(t(), Util.Time.time_of_day()) :: boolean() | |
def for_now?(piece, now_time_of_day) do | |
piece.start_time <= now_time_of_day and piece.end_time > now_time_of_day | |
@spec active_at_time?(t(), Util.Time.time_of_day()) :: boolean() | |
def active_at_time?(piece, time_of_day) do | |
piece.start_time <= time_of_day and time_of_day < piece.end_time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of like current
rather than active
, since these represent scheduled service that may or may not actually be active in the sense of having a vehicle running them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just renamed it to current?
37da3be
to
93d6944
Compare
Coverage of commit
|
Not a huge deal, but before merging this... I noticed that at least one of the functions that I renamed here didn't seem to have any callers, and I kind of want to do another pass just to see if we can clean up some dead code while we're here. |
A newer version of credo exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged. |
@spec is_active(t(), Util.Time.time_of_day(), Util.Time.time_of_day()) :: boolean() | ||
def is_active(trip, start_time_of_day, end_time_of_day) do | ||
@spec active?(t(), Util.Time.time_of_day(), Util.Time.time_of_day()) :: boolean() | ||
def active?(trip, start_time_of_day, end_time_of_day) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one might not be called anywhere - renaming it yielded no compilation errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -75,7 +75,7 @@ defmodule Skate.Settings.User do | |||
Skate.Repo.all(from(u in users_for_route_ids_query(route_ids), select: u.id)) | |||
end | |||
|
|||
def is_in_test_group(user_id, test_group_name) do | |||
def in_test_group?(user_id, test_group_name) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is also dead code (it's only invoked in tests).
That's true as of removing the old search maps code, because I know we were using this to surface some data conditional on a user being in one or the other test group.
I'm on the fence about keeping it though, because it's not exactly dead code - just sleeping - and we will probably wake it up again during detour drawing. We could also use the all_test_group_names
function to get the same information, and it would be better to only have this logic in one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumps [credo](https://github.com/rrrene/credo) from 1.7.1 to 1.7.2. - [Changelog](https://github.com/rrrene/credo/blob/master/CHANGELOG.md) - [Commits](https://github.com/rrrene/credo/commits) --- updated-dependencies: - dependency-name: credo dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
c2786c2
to
6809a33
Compare
Coverage of commit
|
Bumps credo from 1.7.1 to 1.7.2.
Changelog
Sourced from credo's changelog.
Commits
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions
will show all of the ignore conditions of the specified dependency@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)