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.

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

Decouple writefreely library from executable #102

Merged
merged 20 commits into from Jun 14, 2019
Merged

Decouple writefreely library from executable #102

merged 20 commits into from Jun 14, 2019

Conversation

@thebaer
Copy link
Member

thebaer commented May 10, 2019

This exposes setup and admin functions in the writefreely package, and
uses them in the main application / executable, initialized by the flag parsing that
now happens there.

This is the first step towards making writefreely more usable as a
standalone package, and is still a work in progress.

This exposes setup and admin functions in the writefreely package, and
uses them in the main application, initialized by the flag parsing that
now happens there.

This is the first step towards making `writefreely` usable as a
standalone package.
@thebaer thebaer added this to the 0.10 milestone May 10, 2019
@thebaer thebaer changed the base branch from master to develop May 10, 2019
thebaer added 17 commits May 12, 2019
Now the func takes a username and password instead of a single string.
This adds a new `landing` value in the [app] section of config.ini.
If non-empty, unauthenticated users on multi-user instances will be
redirected to the path given there.

This closes T574
This adds a new `landing` value in the [app] section of config.ini.
If non-empty, unauthenticated users on multi-user instances will be
redirected to the path given there.

This closes T574
This ensures the writefreely pkg can be used in other applications that
need to load mysql themselves -- this can be done by building with the
tag: wflib

Ref T613
(instead of writing out the logic of that helper function)

Ref T613
- Adds a new interface, Apper, that enables loading and persisting
  instance-level data in new ways
- Converts some initialization funcs to methods
- Exports funcs and methods needed for intialization
- In general, moves a ton of stuff around

Overall, this should maintain all existing functionality, but with the
ability to now better manage a WF instance.

Ref T613
This ensures outside application builds will succeed when including the
writefreely pkg.

Ref T613
Ref T613
@thebaer thebaer requested a review from robjloranger Jun 14, 2019
@thebaer
Copy link
Member Author

thebaer commented Jun 14, 2019

All set for review. Again this is just a ton of refactoring, so we're mostly concerned with making sure nothing broke in the process. Issues are most likely to happen in components that need initialization, like:

  • templates
  • encryption keys (for session cookies and email addresses stored in database)
  • sessions (e.g. not being able to log in)
  • the Reader section
  • form encoder / decoder (handles API requests)
app.go Show resolved Hide resolved

// Generate keys
initKeyPaths(app)
var keyErrs error

This comment has been minimized.

Copy link
@robjloranger

robjloranger Jun 14, 2019

Member

Should we use a multierror here? Otherwise we would only report as many as the last of three errors back to the caller.

This comment has been minimized.

Copy link
@thebaer

thebaer Jun 14, 2019

Author Member

Possibly.. what's a multierror? I'm not familiar with it.

This comment has been minimized.

Copy link
@robjloranger

robjloranger Jun 14, 2019

Member

It's a hashicorp library, we could use or implement something simliar. not a msut have but it's nice. https://github.com/hashicorp/go-multierror

This comment has been minimized.

Copy link
@robjloranger

robjloranger Jun 14, 2019

Member

note: if you go with that don't forget key/key.go

This comment has been minimized.

Copy link
@thebaer

thebaer Jun 14, 2019

Author Member

👍 thanks, that could definitely be the way to go. I added a TODO in both places so we can be sure to tackle this in another iteration.

@robjloranger
Copy link
Member

robjloranger commented Jun 14, 2019

:lgtm: 👍

thebaer added 2 commits Jun 14, 2019
This moves `hostName` to the `Collection` struct, where it's needed. The
field is populated after successful `GetCollection...()` calls.

This isn't the cleanest way to do things, but it accomplishes the goal.
Eventually, we should accept the AppCfg to `GetCollection...()` calls,
or make them `App` methods, instead of `datastore` methods.

Ref T613
@thebaer thebaer force-pushed the librarization branch from 97ac950 to 36b160b Jun 14, 2019
@thebaer
Copy link
Member Author

thebaer commented Jun 14, 2019

Thanks for reviewing! Merging now.

@thebaer thebaer merged commit ac7d727 into develop Jun 14, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@thebaer thebaer deleted the librarization branch Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.