Skip to content
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

Add Spamtracker from https://github.com/ferrybig/SpamTrackerReboot.git #74

Merged
merged 7 commits into from Apr 26, 2017

Conversation

ferrybig
Copy link
Member

This pull request adds my Spamtracker userscript to the global charcoal userscript repo.

The purpose of this userscript is to alert charcoal users using sound and a desktop notification of new Smokedetector messages that require their attention, it by default uses the meta SE sound, but this can be configured when pressing the link in the footer of the chat page. This userscript can be compared to the Spamtracker Google Chrome extension, that Normal Human had made, but got removed for unknown reasons.

I already have agreed to license my past and future contributions under MIT/Apache-2.0

j-f1
j-f1 previously requested changes Apr 25, 2017
Copy link
Contributor

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

Please fix the lint errors. If you clone this to your computer, running npm install and npm test -- --fix will fix most of them.

@j-f1
Copy link
Contributor

j-f1 commented Apr 25, 2017

I can do a custom merge this evening or tomorrow that will preserve your repo’s history, if you’d like.

@j-f1
Copy link
Contributor

j-f1 commented Apr 25, 2017

You can move the CSS to a file, and it will show up at https://charcoal-se.org/userscripts/spamtracker/spamtracker.css. You can even use SCSS!

@ferrybig
Copy link
Member Author

Is there a way we can make our linting work either with 4 spaces, or tabs, the indentation restriction that the linting is giving me trouble when I need to maintain the script, since it's hard to see what's going on when everything is scrammed to the left of the page

In linting, the following error are still unsolved, even after the --fix command

  • * was used before it was defined.
    Functions are ordered by purpose, not by usage, for example the configuration load and save is in the bottom, and all dom related structure are near each other, only after everything is defined, then it will be loaded

  • Identifier GM_getValue is not in camel case.

  • A function with a name starting with an uppercase letter should only be used as a constructor.
    I didn't define these funtions, these come from the tamper monkey and grease monkey api's

  • domTabSound is defined but never used.
    This will be populated in a future version, defining the variable now makes it clear what the purpose is going to be of the other tab variable

  • cause is defined but never used.
    This variable is only shown as indication that a load error has a cause event, at the moment it is true that it is unused, but later versions are going to show it more clear to the user

  • verifySoundName is assigned a value but never used.
    Also planned for future, was in use during testing, and is going to be used in near future when custom sounds are going to be added, it is just part of the private api of Spamtracker

  • Script URL is a form of eval.
    Se requires a non-empty href on a a element before it renders with the proper styles

  • This line has 2 statements. Maximum allowed is 1.
    --fix changed the original line to have 2 statements

@ArtOfCode-
Copy link
Member

@ferrybig You can override any of xo's settings by creating a package.json file at the root of your subdirectory. Here's metapi's, as an example. Inside xo.rules, you can configure each rule individually. The format is 'rule-name-here': <config>. Usually, passing a 0 as config will turn the rule off.

@j-f1
Copy link
Contributor

j-f1 commented Apr 26, 2017

@ferrybig I’ve fixed all of the lint issues.

@ArtOfCode- ArtOfCode- dismissed j-f1’s stale review April 26, 2017 11:02

because reasons

@ArtOfCode- ArtOfCode- merged commit 8376a06 into Charcoal-SE:master Apr 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants