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

Finish the code reorg #2832

Closed
3 of 5 tasks
paulmelnikow opened this issue Jan 21, 2019 · 5 comments
Closed
3 of 5 tasks

Finish the code reorg #2832

paulmelnikow opened this issue Jan 21, 2019 · 5 comments
Labels
developer-experience Dev tooling, test framework, and CI

Comments

@paulmelnikow
Copy link
Member

paulmelnikow commented Jan 21, 2019 •
edited
Loading

I've gotten a good bit of the way through the work of implementing #2698 but I'm running out of steam so thought I'd pause. I could pick it up later or maybe other folks would like to pick up what's left.

After #2829 and #2831 are merged here's what remains:

#2698 can be used as a reference.
@paulmelnikow paulmelnikow mentioned this issue Jan 21, 2019
Closed
@chris48s
Copy link
Member

After we merged the last refactoring PR I realised that we didn't update https://github.com/badges/shields/blob/master/now.json to account for the new layout. Can we squeeze that change into #2829 or #2831 ahead of review.

@chris48s chris48s added the developer-experience Dev tooling, test framework, and CI label Jan 21, 2019
@paulmelnikow
Copy link
Member Author

Added to #2829.

paulmelnikow added a commit that referenced this issue Jan 24, 2019
@paulmelnikow
Refactor service loader; test on [static dynamic endpoint]
f529ed7 
This moves the loader code into `core/base-service`, leaving behind in `services/index.js` only the convenience imports.

Ref #2832
@paulmelnikow paulmelnikow mentioned this issue Jan 24, 2019
Merged
paulmelnikow added a commit that referenced this issue Jan 25, 2019
@paulmelnikow
Refactor service loader (#2861)
be7cb93 
This moves the loader code into `core/base-service`, leaving behind in `services/index.js` only the convenience imports.

Ref #2832
paulmelnikow added a commit that referenced this issue Feb 25, 2019
@paulmelnikow
Reorganize categories code
2a10641 
Seems that the list of categories belongs with the services, while the
support code belongs with base service.

Ref #2832
@paulmelnikow paulmelnikow mentioned this issue Feb 25, 2019
Merged
paulmelnikow added a commit that referenced this issue Feb 25, 2019
@paulmelnikow
Move "good" badge helpers fro lib/ to services/
561f77f 
Ref #2832
@paulmelnikow paulmelnikow mentioned this issue Feb 25, 2019
Merged
paulmelnikow added a commit that referenced this issue Feb 25, 2019
@paulmelnikow
Reorganize categories code (#3100)
b14fdd7 
Seems that the list of categories belongs with the services, while the
support code belongs with base service.

Ref #2832
paulmelnikow added a commit that referenced this issue Feb 28, 2019
@paulmelnikow
Move "good" badge helpers from lib/ to services/ (#3101)
fafb22e 
This moves a few helpers from `lib/` to `services/`:

build-status.js
build-status.spec.js
color-formatters.js
color-formatters.spec.js
contributor-count.js
licenses.js
licenses.spec.js
php-version.js
php-version.spec.js
text-formatters.js
text-formatters.spec.js
version.js
version.spec.js

And one from `lib/` to `core/`:

unhandled-rejection.spec.js

The diff is long, but the changes are straightforward.

Ref #2832
This was referenced Mar 5, 2019
Merged
Merged
@paulmelnikow
Copy link
Member Author

I started moving more of the legacy helpers, though it seems like it makes more sense to close out the home stretch of refactors in the next couple weeks. This is generating a bunch of conflicts with those changes, and mostly involves changing code that we want to delete.

After #3162 and #3163, what's left is:

  1. badge-data.js which can be removed as soon as the refactors are done
  2. load-logos.js, load-simple-icons.js, logos.js, and svg-helpers.js which should be moved into the npm package. "Refactor logo resolution so it happens in the badge generator" is in the backlog though it doesn't have an issue yet. Probably makes sense to move it once instead of twice.
  3. server-secrets.js which should be eliminated in favor of injecting the secrets through BaseService.register() That'll require touching all the service implementations and probably that should happen once instead of twice.

We could open new issues for 2 and 3 and close this out.

@paulmelnikow
Copy link
Member Author

#3392 removes badge-data.js.

@paulmelnikow paulmelnikow mentioned this issue Apr 29, 2019
Closed
@paulmelnikow
Copy link
Member Author

Item (2) has been listed as a dependency in #3370, so let's track that there.

For item (3) I opened #3393.

Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
developer-experience Dev tooling, test framework, and CI
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@paulmelnikow @chris48s