Skip to content

Filters updated#976

Merged
ArtOfCode- merged 90 commits intocodidact:developfrom
eip-ewi:filters-updated
May 5, 2023
Merged

Filters updated#976
ArtOfCode- merged 90 commits intocodidact:developfrom
eip-ewi:filters-updated

Conversation

@Taeir
Copy link
Contributor

@Taeir Taeir commented Jan 21, 2023

All credit for this feature goes to @MoshiKoi

I have only addressed some of the PR comments and fixed some minor issues as indicated in the original PR #890.

Since I am not able to add changes directly to someone else's branch/fork of the repo, I had to create a new branch and PR.

@Taeir
Copy link
Contributor Author

Taeir commented Jan 21, 2023

@ArtOfCode- With the changes in this PR, search_helper becomes longer than the currently set maximum of 200 lines for a Module. I feel like splitting it up into multiple modules is not really going to improve anything. How should I address this? I can:

  • Exclude this particular file
  • Increase the max module length limit

Or perhaps you have other suggestions?

@ArtOfCode-
Copy link
Member

@Taeir I'd increase the limit. 200 is probably a number mostly out of thin air - in this case it's not enough, so let's make it more.

@MoshiKoi MoshiKoi mentioned this pull request Jan 25, 2023
@Taeir Taeir marked this pull request as ready for review January 25, 2023 15:32
@Taeir Taeir requested a review from ArtOfCode- January 25, 2023 15:32
Copy link
Member

@ArtOfCode- ArtOfCode- left a comment

Choose a reason for hiding this comment

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

LGTM, approving, not merging right now because of deploy sequencing.

@cellio
Copy link
Member

cellio commented Feb 14, 2023

@ArtOfCode- is the "deploy sequencing" concern still an issue?

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.

4 participants

Comments