Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

centralise routing #16

Merged
merged 1 commit into from
Feb 5, 2019
Merged

centralise routing #16

merged 1 commit into from
Feb 5, 2019

Conversation

damienomurchu
Copy link
Contributor

@damienomurchu damienomurchu commented Jan 30, 2019 •
edited

Motivation

Add dedicated routing file for endpoints https://issues.jboss.org/browse/AEROGEAR-8456

What

Routing added & wired up, and endpoint prefix /api applied to all routes, ie localhost:3000/api/apps

Why

Support addition of endpoints

How

Add routing file & api prefix to to be applied to routes

Verification Steps

cd cmd/mobile-security-service
go run main.go
curl localhost:3000/api/apps

Checklist:

  • Code has been tested locally by PR requester
  • Changes have been successfully verified by another team member

Progress

  • Finished task
  • TODO

Additional Notes

@craicoverflow craicoverflow mentioned this pull request Jan 30, 2019
Merged
4 tasks
austincunningham
austincunningham reviewed Jan 31, 2019
View reviewed changes
Copy link
Member

@austincunningham austincunningham left a comment

Choose a reason for hiding this comment

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

Looks like you need a rebase

@damienomurchu
Copy link
Contributor Author

@austincunningham yeah, I'm working off unmerged changes from other PR's, so this branch will often be out of sync :)

@damienomurchu damienomurchu force-pushed the add-routing branch 2 times, most recently from 340e596 to d6b5e3d Compare January 31, 2019 14:56
@damienomurchu damienomurchu changed the title [WIP] add routing [WIP] centralise routing Jan 31, 2019
@damienomurchu damienomurchu force-pushed the add-routing branch 7 times, most recently from 9d49fb0 to 788b122 Compare February 5, 2019 12:20
@damienomurchu damienomurchu changed the title [WIP] centralise routing centralise routing Feb 5, 2019
@damienomurchu damienomurchu force-pushed the add-routing branch 2 times, most recently from ef626e9 to fc2e47e Compare February 5, 2019 14:22
@coveralls
Copy link

coveralls commented Feb 5, 2019 •
edited

Coverage Status

Coverage increased (+0.5%) to 69.403% when pulling 277224e on damienomurchu:add-routing into e485942 on aerogear:master.

@damienomurchu
Copy link
Contributor Author

ready to test

@damienomurchu damienomurchu force-pushed the add-routing branch 5 times, most recently from 307958e to 817d16b Compare February 5, 2019 15:06
davidffrench
davidffrench previously requested changes Feb 5, 2019
View reviewed changes
Copy link
Member

@davidffrench davidffrench left a comment

Choose a reason for hiding this comment

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

Great work Damien. Some small changes needed.

pkg/web/router.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
cmd/mobile-security-service/main.go Outdated Show resolved Hide resolved
cmd/mobile-security-service/main.go Outdated Show resolved Hide resolved
davidffrench
davidffrench reviewed Feb 5, 2019
View reviewed changes
pkg/config/config.go Outdated Show resolved Hide resolved
craicoverflow
craicoverflow reviewed Feb 5, 2019
View reviewed changes
pkg/web/router.go Outdated Show resolved Hide resolved
craicoverflow
craicoverflow reviewed Feb 5, 2019
View reviewed changes
pkg/web/router.go Outdated Show resolved Hide resolved
@craicoverflow
Copy link

You should include the ticket in the commit message

@damienomurchu damienomurchu dismissed davidffrench’s stale review February 5, 2019 16:04

Comments actioned now so dismissing review

@damienomurchu
Copy link
Contributor Author

@craicoverflow will add ticket ref when I squash 👍

davidffrench
davidffrench previously requested changes Feb 5, 2019
View reviewed changes
Copy link
Member

@davidffrench davidffrench left a comment

Choose a reason for hiding this comment

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

Looking very good now @damienomurchu . Only other change needed now is the commit message

@damienomurchu damienomurchu dismissed davidffrench’s stale review February 5, 2019 16:22

Commit message fixed now

@damienomurchu damienomurchu merged commit b5b6bc3 into aerogear:master Feb 5, 2019
@damienomurchu damienomurchu deleted the add-routing branch February 5, 2019 16:30
@damienomurchu damienomurchu restored the add-routing branch February 14, 2019 11:06
jhellar pushed a commit to jhellar/mobile-security-service that referenced this pull request Nov 13, 2019
@camilamacedo86
Merge pull request aerogear#16 from camilamacedo86/FIX
9726aed 
fix(makefile): Undeploy commands do not stop when find errors
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@craicoverflow craicoverflow craicoverflow left review comments

@austincunningham austincunningham austincunningham left review comments

@davidffrench davidffrench davidffrench approved these changes

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@damienomurchu @coveralls @craicoverflow @davidffrench @austincunningham