search

DukeLupus / dlFilter

dlFilter is a text filtering script for mIRC. It is created with both chat and file sharing channels in mind. dlFilter removes ads, requests, annoying KeepTrack, mp3 play & away messages and much more. Also, dlFilter can send notices from fileservers to separate window and group @find results, allowing them to be easily viewed.
Mozilla Public License 2.0
35 stars 1 forks source link

dlFilter v1.17 #7

Closed Sophist-UK closed 7 years ago

Sophist-UK commented 7 years ago

I have reverted the master branch and put my changes into a pull request instead, breaking it up into smaller commits that can more easily be code reviewed.

I have also added a ToDo list to the change log comment block to show what is still planned.

Sophist-UK commented 7 years ago

@SanderSade Feedback welcome. The enable / disable by group functionality may not have all the right event blocks in it, but aside from that this should be fully functional.

SanderSade commented 7 years ago

Going to look at the PR in a few, but I would go with v1.2 or even v2.0 for next release. Your fixes and updates merit a major update label, esp. when going with semver guidelines.

SanderSade commented 7 years ago

I left some comments:

Sophist-UK commented 7 years ago

Review comments addressed (mostly).

Leaving as 1.17 for now - so we can have a v2 beta. But I agree in principle to changing to v2.

I prefer changelog in the script - at least whilst developing, and indeed I may make this visible in the About dialog when I write it. Perhaps we can copy it to ReadMe.md when we publish it as a release?

Sophist-UK commented 7 years ago

P.S. Are you OK with the coding style?

Sophist-UK commented 7 years ago

I didn't think it was quite ready to merge, but if you want to change the version on your web site and point people to it, it can probably be released as beta.

Sophist-UK commented 7 years ago

Please ignore this - I was mistaken about how it was merged.

P.S. In the future it might be better to merge as a sequence of commits rather than as a merged single commit.

If we ever need to track down the commit that has introduced a bug using git bisect, having separate commits would be useful.

SanderSade commented 7 years ago

Coding style seems to be OK - at least I didn't notice anything unseemly...

On commits - I prefer issue-based commits or PR's after initial major changes. That way there is a documented "trail" of changes and reasoning. But I am probably overthinking this for small things like dlf.

On beta - I would prefer not to have everybody switch to beta, but have some users ("testers") to try it first for a while. These are major changes, and if something rarely used/tested, like ops chat, breaks completely...

I probably can ask some people to try it out for a few days, but you have probably more options to do so, as I hardly ever use IRC these days.

Sophist-UK commented 7 years ago

My changes are not yet finished. So we can wait for a couple more weeks whilst I finish them.

Also, when you rewrite the ReadMe, can we add some guidelines on the information we need to receive if someone raises a support issue.