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

MF-1065 - Create commands service #1457

Closed
wants to merge 11 commits into from
Closed

Conversation

fbugarski
Copy link
Contributor

@fbugarski fbugarski commented Sep 3, 2021

Signed-off-by: fbugarski [email protected]

Pull request title should be MF-XXX - description or NOISSUE - description where XXX is ID of issue that this PR relate to.
Please review the CONTRIBUTING.md file for detailed contributing guidelines.

What does this do?

Resolves Create a Commands Service #1065

Signed-off-by: fbugarski <[email protected]>
@fbugarski fbugarski requested a review from a team as a code owner September 3, 2021 14:09
Signed-off-by: fbugarski <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2021

Codecov Report

Merging #1457 (a549e98) into master (f4312ae) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1457      +/-   ##
==========================================
- Coverage   70.92%   70.90%   -0.03%     
==========================================
  Files         123      123              
  Lines        9564     9564              
==========================================
- Hits         6783     6781       -2     
- Misses       2254     2256       +2     
  Partials      527      527              
Impacted Files Coverage Δ
things/service.go 72.02% <0.00%> (-1.20%) ⬇️

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 f4312ae...a549e98. Read the comment docs.

Signed-off-by: fbugarski <[email protected]>
@@ -5,7 +5,7 @@ MF_DOCKER_IMAGE_NAME_PREFIX ?= mainflux
BUILD_DIR = build
Copy link
Contributor

Choose a reason for hiding this comment

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

it is not important for this PR but it would be nice to add here MF_RELEASE ?= latest
and then replace

--tag=$(MF_DOCKER_IMAGE_NAME_PREFIX)/$(svc):latest\

with

--tag=$(MF_DOCKER_IMAGE_NAME_PREFIX)/$(svc):$(MF_RELEASE) \

Copy link
Contributor

Choose a reason for hiding this comment

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

@mteodor interesting - please open a new issue for this and send the PR

Command string `json:"command"`
Name string `josn:"name"`
ChannelID string `json:"channel_id"`
ExecuteTime time.Time `json:"execute_time"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be a string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also - Metadata is missing here.

Signed-off-by: fbugarski <[email protected]>
@drasko drasko changed the title MF-1065 - Create command service MF-1065 - Create commands service Oct 7, 2021
Signed-off-by: fbugarski <[email protected]>
Signed-off-by: fbugarski <[email protected]>
Signed-off-by: fbugarski <[email protected]>
@dborovcanin
Copy link
Collaborator

While the idea is still valid, this PR has been stale for a while and it isn't easy to continue to work on it before huge refactoring, at which point it's simpler to rewrite everything. We'll probably use a different approach with workflow pipelines that are on the roadmap instead. I'll close it but keep the issue open and we can eventually reopen it if we decide to continue this work.

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.

5 participants