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

[WIP] Change Access Control verification #80

Merged
merged 6 commits into from
Jan 24, 2017

Conversation

marioidival
Copy link
Contributor

@marioidival marioidival commented Jan 24, 2017

[WIP] Looking for new way to test it.

In manual test the feature it's ok, but it's cool have test code guarantee.

I NEED HELP

#79, #75, #52

Copy link
Member

@avelino avelino left a comment

Choose a reason for hiding this comment

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

Create a middlewares package to put all the middleware, today we have the middleware package cmd (doesn't make sense)

@marioidival marioidival force-pushed the improvements/access-control-middleware branch from 66a2dd0 to 953ad06 Compare January 24, 2017 15:37
@codecov-io
Copy link

codecov-io commented Jan 24, 2017

Current coverage is 86.49% (diff: 100%)

Merging #80 into master will increase coverage by 0.17%

@@             master        #80   diff @@
==========================================
  Files             6          6          
  Lines           753        733    -20   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            650        634    -16   
+ Misses           68         65     -3   
+ Partials         35         34     -1   

Powered by Codecov. Last update 09817fe...953ad06

@@ -1,4 +1,4 @@
package cmd
package middlewares
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's necessary to put the package name as a prefix middlewares_utils, used utils example. The name utils is not very good, we need to think of another name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, middlewares_utils.go isn't good name, but utils.go makes sense

next(rw, rq)
return
}

Copy link
Member

Choose a reason for hiding this comment

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

Why break two lines? Run gofmt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No rules to this, gofmt do nothing 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

sorry, not exist two line, my eyes must have confused

Copy link
Member

@felipeweb felipeweb left a comment

Choose a reason for hiding this comment

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

👍

@avelino avelino merged commit cdd60ee into master Jan 24, 2017
@avelino
Copy link
Member

avelino commented Jan 24, 2017

thank you @marioidival

@avelino avelino deleted the improvements/access-control-middleware branch January 24, 2017 21:21
avelino pushed a commit that referenced this pull request Jul 16, 2020
* Create utils to middleware

* Remove old permission verifications

* Create middleware to AccessControl

* Remove old permission verification in tables.go

* Create package middleware

* Change name of middlewares utils file
avelino pushed a commit that referenced this pull request Jul 16, 2020
* Create utils to middleware

* Remove old permission verifications

* Create middleware to AccessControl

* Remove old permission verification in tables.go

* Create package middleware

* Change name of middlewares utils file
avelino pushed a commit that referenced this pull request Jul 16, 2020
* Create utils to middleware

* Remove old permission verifications

* Create middleware to AccessControl

* Remove old permission verification in tables.go

* Create package middleware

* Change name of middlewares utils file
avelino pushed a commit that referenced this pull request Jul 16, 2020
* Create utils to middleware

* Remove old permission verifications

* Create middleware to AccessControl

* Remove old permission verification in tables.go

* Create package middleware

* Change name of middlewares utils file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants