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.

Sign up for GitHub

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

Jump to bottom

Upgrade rxjs to v6 in finance app #698

Merged
merged 8 commits into from
Mar 15, 2019
Merged

Upgrade rxjs to v6 in finance app #698

merged 8 commits into from
Mar 15, 2019

Conversation

2color
Copy link
Contributor

@2color 2color commented Mar 4, 2019 •
edited
Loading

What

  • Add rxjs as a dependency
  • Improve dev experience by watching the script in dev/watch mode too
  • Make code changes where necessary

2color added 5 commits March 4, 2019 10:54
@2color
finance: add rxjs v6 as an explicit dependency
075172a
@2color
finance: watch background script in dev mode
6f885a3
@2color
finance: migrate background script to rxjs v6
f9b0608 
- use pipe for operators
- remove first/take where the observable returns a single value and
completes because of sendAndObserveResponse
@2color
finance: remove local rxjs package, migrate app
f6b4c4b
@2color
finance: remove first operator where needless
e1e0289 
- Single response observables
@2color 2color requested review from sohkai and kernelwhisperer March 4, 2019 13:15
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 96.124% when pulling e1e0289 on rxjs6-upgrade-finance into 98c1e38 on master.

@coveralls
Copy link

coveralls commented Mar 4, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.8%) to 96.124% when pulling 007619d on rxjs6-upgrade-finance into 98c1e38 on master.

sohkai
sohkai approved these changes Mar 5, 2019
View reviewed changes
Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

💯 🥇 ✅

@2color
Copy link
Contributor Author

2color commented Mar 5, 2019

Awaiting the final decision on renaming of @aragon/client and release to bump the version in this PR.

@2color 2color added the blocked label Mar 5, 2019
@sohkai sohkai removed the blocked label Mar 7, 2019
@2color 2color merged commit d0896d4 into master Mar 15, 2019
@2color 2color deleted the rxjs6-upgrade-finance branch March 15, 2019 10:05
This was referenced Mar 18, 2019
Merged
Merged
ramilexe pushed a commit to ConsiderItDone/aragon-apps that referenced this pull request Dec 9, 2021
@2color
Upgrade rxjs to v6 in finance app (aragon#698)
fffc70e 
* finance: add rxjs v6 as an explicit dependency

* finance: watch background script in dev mode

* finance: migrate background script to rxjs v6

- use pipe for operators
- remove first/take where the observable returns a single value and
completes because of sendAndObserveResponse

* finance: remove local rxjs package, migrate app

* finance: remove first operator where needless

- Single response observables

* Use first instead of take(1)

* chore: update @aragon/client to @aragon/api

* Update imports to new packages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sohkai sohkai sohkai approved these changes

@kernelwhisperer kernelwhisperer Awaiting requested review from kernelwhisperer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@2color @coveralls @sohkai