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

[WIP] Duplication refactoring #173

Closed
wants to merge 10 commits into from
Closed

[WIP] Duplication refactoring #173

wants to merge 10 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 27, 2017 •
edited by ghost

[DELETED]

See PR #174. Don't scroll down for the love of Jesus.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 55.828% when pulling b622924 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 55.828% when pulling b622924 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@ghost
Copy link
Author

ghost commented Dec 27, 2017 •
edited by ghost

Just a thought, would it make sense to disable coveralls on PRs that have [WIP] or some such in the title? @Arthelon

@LordSputnik
Copy link
Member

@naiveaiguy it would, but I don't believe coveralls supports that functionality.

@naiveai
chore(package.json): Add dupreport script
c44a308 
`npm run dupreport` runs jsinspect on src/.
@coveralls
Copy link

coveralls commented Dec 28, 2017 •
edited

Coverage Status

Coverage remained the same at 55.828% when pulling c44a308 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@coveralls
Copy link

coveralls commented Dec 28, 2017 •
edited

Coverage Status

Coverage remained the same at 55.828% when pulling 3fd7598 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@naiveai
refactor(entityRoutes): Remove reduntant prop keys
845b92c 
In the routes for the entites, when generateProps
is done (in /create and /:bbid/edit), there are keys
like this one:

workTypes: res.locals.workTypes

Howver, this is redundant becuase generateProps does
an Object.assign({}, res.locals, ...), which means such
keys are already present, so they can be removed from the code.
@coveralls
Copy link

coveralls commented Dec 28, 2017 •
edited

Coverage Status

Coverage remained the same at 55.828% when pulling 845b92c on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 55.929% when pulling 3e4cad9 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 55.929% when pulling 3e4cad9 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@coveralls
Copy link

coveralls commented Dec 28, 2017 •
edited

Coverage Status

Coverage increased (+0.1%) to 55.929% when pulling 3426afa on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@coveralls
Copy link

coveralls commented Dec 28, 2017 •
edited

Coverage Status

Coverage increased (+0.1%) to 55.963% when pulling 6d8aa47 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@naiveai
refactor(entityRoutes): Add generateEnityProps
cf908a7 
generateEntityProps gives sensible defaults to most `props` values.
@naiveai
refactor(entityRoutes): Add entityEditorMarkup
d1ac2f6 
This function encapsulates the markup requred for creation/editing
of all the entities.
@coveralls
Copy link

coveralls commented Dec 28, 2017 •
edited

Coverage Status

Coverage increased (+0.1%) to 55.963% when pulling d1ac2f6 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@naiveai
refactor(entityRoutes): Add createOrEditHandler-s
a5e051d 
A function called `makeEntityCreateOrEditHandler` encapsulates
the similarities in entity /create/handler and /edit/:bbid/handler-s.
@coveralls
Copy link

coveralls commented Dec 28, 2017 •
edited

Coverage Status

Coverage increased (+0.8%) to 56.578% when pulling a5e051d on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@naiveai
refactor(entityRoutes): extract getDefaultAliasIndex and areaToOption
2d5c8fd 
creator.js and publisher.js both had these, put them in entity.js
@coveralls
Copy link

coveralls commented Dec 28, 2017 •
edited

Coverage Status

Coverage increased (+0.8%) to 56.66% when pulling 2d5c8fd on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@coveralls
Copy link

coveralls commented Dec 29, 2017 •
edited

Coverage Status

Coverage increased (+1.0%) to 56.783% when pulling 019f28b on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@coveralls
Copy link

coveralls commented Dec 29, 2017 •
edited

Coverage Status

Coverage increased (+1.1%) to 56.947% when pulling 2535c2d on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@naiveai
refactor(editor): Extract getIdEditorJSON
df8e369 
There's potential here for more fixing but that would require pretty
much a wholesale refactor of everything about this file.
@coveralls
Copy link

coveralls commented Dec 29, 2017 •
edited

Coverage Status

Coverage increased (+1.2%) to 57.002% when pulling df8e369 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@coveralls
Copy link

coveralls commented Dec 29, 2017 •
edited

Coverage Status

Coverage increased (+1.3%) to 57.101% when pulling bf845c2 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@coveralls
Copy link

coveralls commented Dec 29, 2017 •
edited

Coverage Status

Coverage increased (+1.3%) to 57.101% when pulling 48ff9b1 on naiveaiguy:duplicate-refactor-eshansingh into e613c4c on bookbrainz:master.

@ghost ghost closed this Dec 29, 2017
@ghost ghost deleted the duplicate-refactor-eshansingh branch December 29, 2017 06:12
@ghost ghost mentioned this pull request Dec 29, 2017
Merged
@ghost
Copy link
Author

ghost commented Dec 29, 2017

See #174 for a PR that's not an abomination. It's for cases like this that I wish GitHub would allow us to delete PRs.

Mistakes were made.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@coveralls @LordSputnik @naiveai