Financial-Times / tako

🐙 A GitHub App that provides an API listing the repositories it is installed on.
MIT License
4 stars 0 forks source link

Refactor Tako #36

Closed sjparkinson closed 5 years ago

sjparkinson commented 5 years ago

Best to not review this one side-by-side. Have a browse of the branch.

I've reworked most of @simonplend's logic, and have simplified the store of repositories down to a Map, for now.

~I'm not happy with having app.repositoryStore as a thing, but it does actually work.~

I did try adding to context using something like...

const repositoryStore = new Map();

app.on('*', context => { context.repositoryStore = repositoryStore; });

... but that doesn't actually work, the modified context isn't available in other handlers for some reason. Ideas welcome 💡.

There is an initial trio of tests, but I still need to write tests for...


The idea is not to refresh on requests to the API, but to keep the list of repositories up-to-date by adding and removing them as we get the relevant events from GitHub. Building an initial list of repositories by calling octokit.apps.getInstallationRepositories when we start the application.


Resolves #3. Resolves #4. Resolves #10. Resolves #6.

Kinda closes #21.

sjparkinson commented 5 years ago

Stuck on https://github.com/probot/probot/issues/760, could sort out a workaround in initalise.js for now.

sjparkinson commented 5 years ago

@simonplend @quarterto this is now in a good place for a review.

codecov[bot] commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@b8a2c16). Click here to learn what that means. The diff coverage is 94%.

Impacted file tree graph

@@          Coverage Diff           @@
##             master   #36   +/-   ##
======================================
  Coverage          ?   94%           
======================================
  Files             ?     4           
  Lines             ?   100           
  Branches          ?     3           
======================================
  Hits              ?    94           
  Misses            ?     6           
  Partials          ?     0
Impacted Files Coverage Δ
src/repositories.js 100% <100%> (ø)
src/index.js 81.81% <81.81%> (ø)
src/initialise.js 96.66% <96.66%> (ø)
src/routes.js 97.82% <97.82%> (ø)

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 b8a2c16...e714745. Read the comment docs.

simonplend commented 5 years ago

What about global.repositoryStore instead? I'm not sure if we can trust app.