Removing unnecessary suffixes

[MikeM711]

Hi, I am a bit of a beginner when it comes to PRs, I hope this one looks OK!

This is a response to issue #69

Inside the dataProcessor.js script, I have created some lines of code that checks and deletes unnecessary suffixes found in the retailer name for deals & cashbacks to enhance the google algorithm as it transforms raw data into JSON

Suffixes include: ['.com',' - Special',' - Featured','®','™' ]

• Ex: "Chilis - Special" → "Chilis"
• Ex: "BestBuy.com®" → "BestBuy"

Some notes:

• I figured the .com in Hotels.com is what the company is known for, so I kept it in, unless otherwise
• Interestingly, in some of the cases where cashback/deal properties have remained unchanged, my google search may pick up different site_url (?)

[MikeM711]

Just noticed my previous commit doesn't seem like it is working as expected... needs fixing...

[MikeM711]

Hopefully that should do it, had a bit of a blunder there, need to take my time on these commits next time 😓... few more things...

[MikeM711]

OK, I think I am done, let me know what you think!

[nareddyt]

Closes #69

Looks awesome! Haha don't worry about having to redo commits. Unfortunately we don't have unit tests for things like this, but I manually tested your changes and they work great.

I'll merge this change in. It should automatically go out to users without needing to release the chrome extension. All users should receive the latest data within a day.

Thanks for making this contribution :)

[MikeM711]

Thank you! This was a fun bugfix!

I googled this a little bit and I wonder if I should "squash commits" next time?

[MikeM711]

...Spoke a little too soon https://github.com/nareddyt/discover-rewards-notifier/blob/master/tools/dataProcessor.js#L234 item.site_name should be item as I noticed in the JSON that Hotels was not Hotels.com😓

apologies over here

I do have the updated fork on my end: https://github.com/nareddyt/discover-rewards-notifier/commit/7c495ff9a694bd51f0eb13889f73ac4334b35c95

Let me know what you want me to do!

[nareddyt]

Wow good catch, I did not notice that either. No worries about it, it's part of software engineer :)

The correct protocol would be to submit a new Pull Request with that bug fix. I made issue #83, so if you want to make a PR referencing that, it would be great. Additionally, maybe we should rename item to itemName in the PR just to make it clearer. Sorry for not catching that!

As for your question about squashing commits, I actually have that option disabled in the repository settings on Github (see picture below). This is actually a pretty controversial topic: Squash merging makes your the history look cleaner, but it means you are effectively changing history. Here's a strongly worded article about it and here's a more balanced discussion.

When I was originally developing this extension, I was making fairly large PRs that has a ton of changes in them, so it made sense to show all my individual commits on master. Since then, I decided to disable squashing to keep all our merges consistent. Hope that explains it!

[MikeM711]

Thank you! This is all really good information & learning the GitHub etiquette!

You're right, I can definitely see how a messy commit history would be less important compared to the privilege of being able to trace the full history of a project... and one bad move may not be worth the "tidiness".