Remove obsolete code #3730
Merged
Remove obsolete code #3730
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
houndci-bot
reviewed
Sep 26, 2019
javierm
force-pushed
the
remove_obsolete_code
branch
from
d3c5a76
to
d79d840
Compare
houndci-bot
reviewed
Sep 27, 2019
houndci-bot
reviewed
Sep 28, 2019
javierm
force-pushed
the
remove_obsolete_code
branch
from
4b64487
to
7373e83
Compare
houndci-bot
reviewed
Sep 29, 2019
javierm
force-pushed
the
remove_obsolete_code
branch
from
d0e5157
to
357bf5f
Compare
Now the test makes more sense, since it checks the third message for the same sender is valid.
The test passed even if proposal recommendations returned archived proposals, because we forgot to add the tag list the user is interested in.
These tests are only checking which proposals are not included in the recommendations, so we don't need to sort the included ones, just like we don't use the cached votes up attribute in the tests preceeding these ones.
This method isn't used since commit e47cbe2, where we replaced it with `headings_voted_within_group`.
The group is automatically assigned when we assign the heading. The budget isn't needed either, except for a special case related to the reason to be rejected.
The file name finished with `_specs.rb` instead of `_spec.rb`, and so it wasn't being tested at all.
This way we can be more flexible about the factory we can pass as parameter.
These specs had never been tested before and were always broken.
We were using the same order for most commented and for newest, so the test wasn't as effective as it could be.
The valuation comment doesn't show up in the comment show action because it's not a child of the parent comment. With a regular comment, the test passes as well. However, if we make the valuation comment a child of the parent comment, it shows up both in the index and show actions. That's because the method `root_descendants` in the `CommentTree` class doesn't filter valuation comments. I'm not sure whether it's a bug or the intended behaviour.
The attribute made sense before we changed it in commit ba1a6b4. Since then, all milestones have the same date, so the attribute doesn't affect the test at all.
Apparently we forgot to use the variables which meant the tests weren't testing exactly what they were supposed to test.
javierm
force-pushed
the
remove_obsolete_code
branch
from
357bf5f
to
82aa8db
Compare
The `create_proposal_notification` method won't create a new notification when a proposal has no supporters, so in the test `notification3` was actually the same as `notification2`, related to `proposal2`.
This is one of the most strange behaviours in ruby: if a variable doesn't exist, assigning to itself will return `nil`. So a line like: mdmkdfm = ooops if mdmkdfm.respond_to?(:uiqpior) Surprisingly will not raise any errors: the nonexistent `mdmkdfm` variable will be evaluated to `nil`, `mdmkdfm.respond_to?(:uiqpior)` will evaluate to `nil.respond_to?(:uiqpior)`, which will return `false`, and then the line will be evaluated as `mdmkdfm = ooops if false`, which will return `nil`. Maybe in the future Ruby will change this behaviour. We hope CONSUL is now in better shape if that ever happens :).
The instance variable was being evaluated to `nil`, and the budget was automatically created by the `set_denormalized_ids` method in the budget investment class.
These factories were only used in one place and they even declared ID attributes. Using the comment factory with the `commentable` attribute does the same thing.
javierm
force-pushed
the
remove_obsolete_code
branch
from
82aa8db
to
f426b25
Compare
|
Travis build failed due to issue #3703; not related to this pull request. |
smarques
pushed a commit
to venetochevogliamo/consul
that referenced
this pull request
Apr 29, 2020
Remove obsolete code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives