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

chore: check ecobalyse-data sync for PR #915

Merged
merged 7 commits into from
Feb 4, 2025
Merged

chore: check ecobalyse-data sync for PR #915

merged 7 commits into from
Feb 4, 2025

Conversation

paulboosz
Copy link
Collaborator

@paulboosz paulboosz commented Jan 30, 2025 •
edited
Loading

🔧 Problem

We have 2 repos ecobalyse-data and ecobalyse so we have out of sync issues.

I experienced them and it's quite annoying.

We agreed upon a workflow with the following rules :

  • ecobalyse-data PR should be merged first
  • Only then can a corresponding syncingecobalyse PR can be merged

With this workflow, ecobalyse-data/main should always be sync with ecobalyse/branch_a when doing a PR on ecobalyse:

🍰 Solution

In every ecobalyse PR add a check that verifies if ecobalyse-data and ecobalyse are in sync for the generated ecobalyse-data files :

  • "public/data/food/ingredients.json"
  • "public/data/food/processes.json"
  • "public/data/textile/materials.json"
  • "public/data/textile/processes.json"
  • "public/data/object/processes.json"

🚨 Points to watch/comments

  • As ecobalyse/master might be temporarily behind ecobalyse-data/main, this check should not apply in the CI on ecobalyse/master. Only on PR to ecobalyse/master

  • If someone merge a json-changing PR in ecobalyse-data and he doesn't merge the corresponding PR on ecobalyse, then this check is going to block all ecobalyse PR because they will all be out of sync. But that's kind of the point, it forces us to always sync ecobalyse-data and ecobalyse

  • I didn't add public/data/textile/processes_impacts.json to the check as it was more complicated with the encryption. Any modification to processes_impacts.json should normally modify processes.json so it's not that important

  • Unit testing check-ecobalyse-data-sync.sh would be nice

🏝️ How to test

Success

  • running ./check-ecobalyse-data-sync.sh should succeed (if this is sync to ecobalyse-data)

Failure

  • add a difference in one of the generated json (listed above)
  • running ./check-ecobalyse-data-sync.sh should fail and display the diff

@paulboosz paulboosz changed the title add script to CI to check ecobalyse-data sync chore: check ecobalyse-data sync for PR Jan 30, 2025
@paulboosz paulboosz marked this pull request as ready for review February 3, 2025 14:06
@paulboosz paulboosz requested review from n1k0 and vjousse February 3, 2025 14:06
vjousse
vjousse reviewed Feb 4, 2025
View reviewed changes
Copy link
Collaborator

@vjousse vjousse left a comment

Choose a reason for hiding this comment

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

Great initiative! I’ve added some comments that I would like to be addressed before merging. Don't hesitate if you want to discuss about it.

n1k0
n1k0 reviewed Feb 4, 2025
View reviewed changes
Copy link
Member

@n1k0 n1k0 left a comment

Choose a reason for hiding this comment

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

Great idea. I've mostly thumbed up @vjousse review comments here.

@paulboosz
Copy link
Collaborator Author

Thanks for the review @vjousse @n1k0.
I implemented the suggested improvements

@vjousse vjousse self-requested a review February 4, 2025 10:39
vjousse
vjousse approved these changes Feb 4, 2025
View reviewed changes
Copy link
Collaborator

@vjousse vjousse left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@paulboosz paulboosz merged commit e45109e into master Feb 4, 2025
7 checks passed
@paulboosz paulboosz deleted the chore/check-ecobalyse-data-sync branch February 4, 2025 14:38
@github-actions github-actions bot mentioned this pull request Feb 4, 2025
Merged
vjousse pushed a commit that referenced this pull request Mar 4, 2025
@github-actions
chore(release): release v4.0.0 (#909)
98cd00b 
## [4.0.0](https://github.com/MTES-MCT/ecobalyse/compare/v3.1.0..v4.0.0)
(2025-03-03)



### 🚀 Features

- Add pre-treatments and update bleaching process
([#898](#898))
- *(textile,ui)* Apply default trims on product category change
([#910](#910))
- Add link to docs in trims section.
([#911](#911))
- Update finishing
([#906](#906))
- *(textile)* Add pre-treatments at the ennobling step.
([#916](#916))
- Update aquatic pollution and pre-treatments computations
([#928](#928))
- [**breaking**] Replace dyeing medium parameter with dyeing process
type. ([#941](#941))
- *(food)* Add transport cooling column to ingredients explorer.
([#950](#950))

### 🪲 Bug Fixes

- Include trims impacts to score without durability.
([#912](#912))
- *(security)* Upgrade sentry libs to v8.49.0
([#918](#918))
- Remove the sourceId from the explorer
([#947](#947))

### 📚 Documentation

- Add FAQ entry about security & self-hosting.
([#919](#919))

### ⚙️ Miscellaneous Tasks

- *(data)* Update fast fashion examples nb of references.
([#908](#908))
- For bleaching set etf to 0
([#914](#914))
- Check ecobalyse-data sync for PR
([#915](#915))
- Sync ecobalyse-data after bw update
([#920](#920))
- Doubts on the lamb, hide it for now
([#927](#927))
- Upgrade dependencies, 2025, Feb 12.
([#938](#938))
- *(textile)* Remove obsolete waste for material
([#940](#940))
- Use new deployment stack `scalingo-22`
([#939](#939))
- WFLDB export from simapro
([#942](#942))
- Sync from ecobalyse-data#48
([#944](#944))
- Update wool "nouvelle filière" with new impacts
([#943](#943))
- Convert to camelCase json keys
([#946](#946))
- Enable all verticals in review apps
([#953](#953))
- Update ingredient name in score history
([#948](#948))

<!-- generated by git-cliff -->

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Mar 4, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n1k0 n1k0 n1k0 left review comments

@vjousse vjousse vjousse 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.
3 participants
@paulboosz @n1k0 @vjousse