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

NOISSUE - Fix bootstrap token naming and interfaces named args #1117

Merged
merged 7 commits into from
Apr 16, 2020
Merged

NOISSUE - Fix bootstrap token naming and interfaces named args #1117

merged 7 commits into from
Apr 16, 2020

Conversation

manuio
Copy link
Contributor

@manuio manuio commented Apr 15, 2020

Signed-off-by: Manuel Imperiale manuel.imperiale@gmail.com

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
@manuio manuio requested a review from a team as a code owner April 15, 2020 08:55
if err != nil {
if id == "" {
// Fail silently.
bs.sdk.DeleteThing(cfg.MFThing, key)
bs.sdk.DeleteThing(cfg.MFThing, token)

Choose a reason for hiding this comment

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

Error return value of bs.sdk.DeleteThing is not checked (from errcheck)

if err != nil {
if err == mfsdk.ErrNotFound {
return mfsdk.Thing{}, errors.Wrap(errThingNotFound, ErrNotFound)
}

if id != "" {
bs.sdk.DeleteThing(thingID, key)
bs.sdk.DeleteThing(thingID, token)

Choose a reason for hiding this comment

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

Error return value of bs.sdk.DeleteThing is not checked (from errcheck)

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

// RetrieveByID retrieves the Config having the provided identifier, that is owned
// by the specified user.
RetrieveByID(string, string) (Config, error)
RetrieveByID(key, id string) (Config, error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider renaming key to owner in repository methods (both interface and implementation, but in tests also).

mteodor
mteodor previously approved these changes Apr 15, 2020
Copy link
Contributor

@mteodor mteodor left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
@codecov-io
Copy link

codecov-io commented Apr 15, 2020

Codecov Report

Merging #1117 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1117   +/-   ##
=======================================
  Coverage   76.12%   76.12%           
=======================================
  Files          96       96           
  Lines        6890     6890           
=======================================
  Hits         5245     5245           
  Misses       1295     1295           
  Partials      350      350           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bd7457...bae7230. Read the comment docs.

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
@@ -57,7 +57,7 @@ func NewConfigRepository(db *sqlx.DB, log logger.Logger) bootstrap.ConfigReposit
return &configRepository{db: db, log: log}
}

func (cr configRepository) Save(cfg bootstrap.Config, connections []string) (string, error) {
func (cr configRepository) Save(cfg bootstrap.Config, chsConnIDs []string) (string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider renaming chsConnIDs.

@manuio manuio merged commit 7d839b7 into absmach:master Apr 16, 2020
@manuio manuio deleted the bootstrap branch April 16, 2020 10:33
manuio added a commit that referenced this pull request Oct 12, 2020
* NOISSUE - Fix bootstrap token naming and interfaces named args

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Fix CI bot

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Use owner for repository layer

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Fix reviews

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
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.

6 participants