Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Give more meaningful name to siteUtils method #3640

Merged
merged 1 commit into from
Sep 3, 2016
Merged

Give more meaningful name to siteUtils method #3640

merged 1 commit into from
Sep 3, 2016

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented Sep 1, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Ran git rebase -i to squash commits if needed.

clearSitesWithoutTags => clearHistory

Fixes #3639

Auditors: @bbondy @bridiver

Test Plan:

  • Visit about:history
  • Use the "Clear browsing data button"
  • Ensure "Browsing history" is on and click "OK"
  • Go back to about:history
  • Verify history is cleared

* @param sites The application state's Immutable sites list.
*/
module.exports.clearSitesWithoutTags = function (sites) {
module.exports.clearHistory = function (sites) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this clears history for all sites we should use the method here https://github.com/brave/browser-laptop/blob/master/app/sessionStore.js#L258

@bridiver
Copy link
Collaborator

bridiver commented Sep 2, 2016

can you squash and then I think this is good to merge

clearSitesWithoutTags => clearHistory

Fixes #3639

Auditors: @bbondy

Test Plan:
- Visit about:history
- Use the "Clear browsing data button"
- Ensure "Browsing history" is on and click "OK"
- Go back to about:history
- Verify history is cleared

2) Updated sessionStore.cleanAppData to use siteUtil.clearHistory

NOTE: Conversion to Immutable happens because siteUtil assumes all input is Immutable.
appState is already converted to a regular JS object when it reaches cleanAppData.

Auditors: @bridiver

Test Plan:
- Ensure history is set to clear on exit
- Open a few sites
- Exit Brave
- Launch Brave and verify sites are removed
@bsclifton
Copy link
Member Author

@bridiver squashed 😄

@bbondy
Copy link
Member

bbondy commented Sep 3, 2016

nice cleanup, thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants