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

[FIX] web_widget_json_graph: Remove scss file #26

Merged
merged 1 commit into from
Aug 31, 2020

Conversation

frahikLV
Copy link

@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2020

Codecov Report

Merging #26 into 13.0 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             13.0      #26   +/-   ##
=======================================
  Coverage   77.51%   77.51%           
=======================================
  Files          12       12           
  Lines         258      258           
=======================================
  Hits          200      200           
  Misses         58       58           

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 7684de3...b5eb504. Read the comment docs.

@tomeyro tomeyro merged commit b857e83 into Vauxoo:13.0 Aug 31, 2020
@luisg123v luisg123v deleted the 13.0-fix_widget_json-dev-frahik branch September 6, 2020 17:04
@luisg123v
Copy link

@frahikLV @tomeyro

You have caused this branch to diverge by merging this PR. PRs should target the OCA repo instead.

Even if it was a temporary fix, a definitive PR should've been created. I advised @frahikLV to announce on OCA#1388 he was going to migrate it.

CC @moylop260 @hbto

@luisg123v
Copy link

Sorry @frahikLV, but I had to undo this change as well as #25. We need to avoid divergences because of:

  • It makes harder for us to keep repos updated with the original ones
  • If someone else needs the repo, they will have to migrate it again (duplicated work)
  • If it's on the OCA, both us and the community could apply or benefit from fixes

CC @hugho-ad

@hugho-ad
Copy link

hugho-ad commented Sep 7, 2020

@frahikLV

Please added as a patch until you open the mr to OCA and get's merged

@luisg123v
Copy link

@hugho-ad,

@frahikLV is planning on adding this module directly into the forecast repo. That's because it's the only repo that uses it, and it's not even migrated to v12 in the OCA, it was also merged for vauxoo:v12 on #22.

What do you think?

@hugho-ad
Copy link

hugho-ad commented Sep 7, 2020

I would rather try to move it to OCA

@luisg123v
Copy link

I agree.

@frahikLV I merged the change in the forecast repo for now so we don't get stuck, but please add to your ToDo list creating the PR to the OCA for v12 & v13.

If you're too busy, I can help with that.

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