Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Closes #1340 - Add HistoryState to content state #7024

Merged
merged 1 commit into from
May 22, 2020

Conversation

NotWoods
Copy link
Contributor


Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

@codecov
Copy link

codecov bot commented May 19, 2020

Codecov Report

Merging #7024 into master will decrease coverage by 0.06%.
The diff coverage is 78.94%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7024      +/-   ##
============================================
- Coverage     77.16%   77.09%   -0.07%     
- Complexity     4632     4774     +142     
============================================
  Files           628      627       -1     
  Lines         22958    23475     +517     
  Branches       3342     3434      +92     
============================================
+ Hits          17716    18099     +383     
- Misses         3856     3951      +95     
- Partials       1386     1425      +39     
Impacted Files Coverage Δ Complexity Δ
...mozilla/components/concept/engine/EngineSession.kt 75.34% <0.00%> (-0.52%) 3.00 <0.00> (ø)
...a/components/concept/engine/history/HistoryItem.kt 33.33% <33.33%> (ø) 1.00 <1.00> (?)
...omponents/browser/session/engine/EngineObserver.kt 70.52% <75.00%> (+0.19%) 51.00 <1.00> (+1.00)
...a/components/browser/state/action/BrowserAction.kt 94.28% <100.00%> (+0.21%) 0.00 <0.00> (ø)
...nents/browser/state/reducer/ContentStateReducer.kt 81.08% <100.00%> (+3.30%) 26.00 <0.00> (+1.00)
...lla/components/browser/state/state/ContentState.kt 100.00% <100.00%> (ø) 19.00 <1.00> (+1.00)
...onents/browser/state/state/content/HistoryState.kt 100.00% <100.00%> (ø) 3.00 <3.00> (?)
...s/lib/push/firebase/AbstractFirebasePushService.kt 64.28% <0.00%> (-3.58%) 9.00% <0.00%> (ø%)
...ents/browser/icons/extension/IconMessageHandler.kt
.../components/browser/icons/extension/IconMessage.kt
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9be6690...1a84e76. Read the comment docs.

@NotWoods NotWoods changed the title Closes #7023 - Add HistoryState to content state Closes #1340 - Add HistoryState to content state May 19, 2020
@jonalmeida jonalmeida linked an issue May 19, 2020 that may be closed by this pull request
Copy link
Contributor

@jonalmeida jonalmeida left a comment

Choose a reason for hiding this comment

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

Did a quick review so far and this looks great! 🎉 Two things left and we can merge:

  • Needs some tests for the EngineSessions and the EngineObserver.
  • Update the issue ID in the comment.

@NotWoods NotWoods marked this pull request as ready for review May 20, 2020 00:47
@NotWoods NotWoods requested a review from jonalmeida May 22, 2020 16:28
@jonalmeida
Copy link
Contributor

@NotWoods Nice! I think we're still missing tests for the GeckoEngineSession before we can merge, those should be straight-forward. 🙂

Copy link
Contributor

@jonalmeida jonalmeida left a comment

Choose a reason for hiding this comment

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

bors r+

@bors
Copy link

bors bot commented May 22, 2020

Build succeeded:

@bors bors bot merged commit 3df24ee into mozilla-mobile:master May 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose "session history" in Session
2 participants