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.

Sign up for GitHub

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

Jump to bottom

First step of simplifaction of task output - update importer package #963

Merged
merged 13 commits into from
May 20, 2019
Merged

First step of simplifaction of task output - update importer package #963

merged 13 commits into from
May 20, 2019

Conversation

krhubert
Copy link
Contributor

I have removed tests that were using external services.

@mesg-bot
Copy link
Member

This pull request has been mentioned on MESG Community. There might be relevant details there:

https://forum.mesg.com/t/2019-05-17-update/282/6

@krhubert krhubert force-pushed the feature/single-output-pt-1 branch from 0d4d94e to c35d5b2 Compare May 17, 2019 13:19
@krhubert krhubert force-pushed the feature/single-output-pt-1 branch 2 times, most recently from 024a10d to 67fbb14 Compare May 17, 2019 14:51
@krhubert krhubert mentioned this pull request May 17, 2019
Merged
antho1404
antho1404 requested changes May 17, 2019
View reviewed changes
Copy link
Member

@antho1404 antho1404 left a comment

Choose a reason for hiding this comment

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

The core cannot start, system services are not valid.

Please do some manual testing

@krhubert
Copy link
Contributor Author

krhubert commented May 17, 2019 •
edited
Loading

I did the tests before and none of the system services will work in that case. @NicolasMahe requested to do PR following the order from the forum https://forum.mesg.com/t/simplification-of-the-tasks-output/265/6 and there update system services are one of the last tasks.

I'm not quite following how then I should divide this work. From one hand the post on the forum says about very small pull request and from the other hand everything should work. It's impossible to keep the order of PRs, small PRs and working PRs at the same time. It's like CAP theorem - only 1 or 2 of those 3 will work at the same time

@antho1404
Copy link
Member

Please have a look at this PR #965, the core is not broken and it's just an update of the mesg.yml like you did for the tests so it's still the purpose of this PR.

Of course, this will still break the marketplace service but I've created a new version of the mesg-js lib https://github.com/mesg-foundation/mesg-js/tree/feature/output-simplification with the tag next so we could just update the marketplace package.json with that version and the pull request still stay small and the core is not broken and works perfectly because the system services are based on success/error outputs already.

The part of the forum to update the services is all the other services not system services. System services are part of the core and are necessary to run the core.

@NicolasMahe
Copy link
Member

@antho1404 Nice catch about the mesg-js that impacted the system service! I forgot about this one.

I tried and everything seems to work correctly 👍

NicolasMahe
NicolasMahe previously approved these changes May 19, 2019
View reviewed changes
@NicolasMahe NicolasMahe mentioned this pull request May 19, 2019
Merged
@NicolasMahe NicolasMahe changed the base branch from dev to feature/single-output May 20, 2019 04:50
@NicolasMahe
Copy link
Member

I changed to base branch of this PR to feature/single-output so we can still release the dev branch in case there is bug fix!

@krhubert
Merge pull request #965 from mesg-foundation/feature/single-output-pt…
edef6f5 
…-1-2

Simplify output for system services
@krhubert krhubert requested review from antho1404 and NicolasMahe and removed request for antho1404 May 20, 2019 06:31
@krhubert krhubert requested a review from antho1404 May 20, 2019 06:31
@antho1404 antho1404 merged commit 957a563 into feature/single-output May 20, 2019
@antho1404 antho1404 deleted the feature/single-output-pt-1 branch May 20, 2019 14:37
@mesg-bot
Copy link
Member

This pull request has been mentioned on MESG Community. There might be relevant details there:

https://forum.mesg.com/t/simplification-of-the-tasks-output/265/6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NicolasMahe NicolasMahe NicolasMahe approved these changes

@antho1404 antho1404 antho1404 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.
4 participants
@krhubert @mesg-bot @antho1404 @NicolasMahe