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

Implement Outputs #140

Merged
merged 2 commits into from
Feb 24, 2021
Merged

Implement Outputs #140

merged 2 commits into from
Feb 24, 2021

Conversation

hoshsadiq
Copy link
Contributor

As discussed in #50, this is a replacement for #51.
I didn't really change much so in terms of the layout it's pretty much the same. I pretty much just rebased, and made a few small changes.

To address a few concerns:

  • In terms of layout, there's not a lot to add. The only thing I could think of was adding a blue header bar to the "outputs for module root", but it didn't seem appropriate.
  • It's hard to not store values as strings, mostly because we do still need strings. So storing data as something other than strings, will be a lot of work.
  • Module outputs do not seem to be available unlike pre-0.12. I think the original code in Add support for outputs (Fix #50) #51 does deal with it, but I've not tested.

Happy to make any additional changes you like.

Fixes #50, closes #51

Copy link
Contributor

@raphink raphink left a comment

Choose a reason for hiding this comment

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

Linting is failing in Travis CI.

Other than that, let's go with this implementation 👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 13.618% when pulling 846c632 on hoshsadiq:outputs into 92eb7ba on camptocamp:master.

@hoshsadiq
Copy link
Contributor Author

Thanks, didn't realise for some reason.

On a side note, following message on travis-ci.org:

Please be aware travis-ci.org will be shutting down in several weeks, with all accounts migrating to travis-ci.com. Please stay tuned here for more information.

Might be worth migrating to something else (note travis-ci.com severely limits build minutes for free usage).

@raphink
Copy link
Contributor

raphink commented Feb 24, 2021

Yes, I'm aware of the Travis CI situation. I need to find time to migrate our projects to GitHub Actions. We have hundreds of repos to migrate though, and only so much time to do it 😁

@raphink raphink merged commit 3bf884d into camptocamp:master Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

View for outputs
3 participants