Page MenuHomePhabricator
Create Task
Maniphest T163274

Move JCMapDataContent from JsonConfig to Kartographer
Open, MediumPublic
Actions

Description

Cross-repo dependencies make maintenance PITA otherwise. Or maybe not cause cross-repo dependencies will be there in any case - needs discussion.

Details

SubjectRepoBranchLines +/-
Zuul: Run all Kartographer tests alongside each patch to JsonConfigintegration/configmaster+1 -1
Customize query in gerrit

Event Timeline

MaxSem created this task.Apr 18 2017, 10:33 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 18 2017, 10:33 PM
MaxSem renamed this task from Move JCMapData content from JsonConfig to Kartographer to Move JCMapDataContent from JsonConfig to Kartographer.Apr 18 2017, 10:33 PM
Yurik subscribed.Apr 19 2017, 4:10 AM

JCMapDataContent relies on both JsonConfig & Kartographer. But while it could still function properly without Kartographer, it cannot function at all without JsonConfig. This is similar to how MediaWiki api uses syntax highlighting - it makes the output nicer, but not required to exist.

debt removed a project: Maps-Sprint.Jun 15 2017, 12:25 AM
debt subscribed.

Moving off the sprint board - the Discovery team won't be able to do this work at this time.

• Mholloway added a project: Product-Infrastructure-Team-Backlog-Deprecated.Oct 2 2018, 3:30 PM
• Mholloway moved this task from Unsorted to General on the Maps (Kartographer) board.
Yurik added a comment.Oct 2 2018, 7:46 PM

See my previous comment, I don't think this task should be done at all.

MSantos subscribed.EditedOct 4 2018, 2:28 PM

Recently we had CI complaining about this cross-repo dependencies and it's now blocking one patch, that's why we brought this to the RI Backlog, so we can investigate. You can see the error below:

18:44:15 Fatal error: Class undefined: Kartographer\SimpleStyleParser in /workspace/src/extensions/JsonConfig/includes/JCMapDataContent.php on line 48

Since moving JCMapDataContent from JsonConfig to Kartographer is not a good idea, would you suggest some other path to follow? To be honest I didn't get the time for further investigations on the code and I don't have an opinion yet.

MSantos added a comment.Oct 4 2018, 2:28 PM
This comment was removed by MSantos.
MaxSem added a comment.Oct 4 2018, 6:44 PM

A dependency can't be avoided here: either JsonConfig needs to depend on Kartographer, or vice versa. CI just needs to be aware of this dependency and always clone both extensions.

MSantos added a comment.Oct 5 2018, 3:06 PM

I see it you had this concern before https://gerrit.wikimedia.org/r/c/integration/config/+/352279

I am going to bring this topic again for discussion and maybe ask for this patch to be merged.

gerritbot subscribed.Oct 5 2018, 3:14 PM

Change 464827 had a related patch set uploaded (by MSantos; owner: MSantos):
[integration/config@master] Allowing JsonConfig to be tested with Kartographer

https://gerrit.wikimedia.org/r/464827

gerritbot added a project: Patch-For-Review.Oct 5 2018, 3:14 PM
MSantos moved this task from Needs triage to Needs investigation on the Product-Infrastructure-Team-Backlog-Deprecated board.Jan 2 2019, 4:46 PM
• Jhernandez moved this task from Needs investigation to Needs triage on the Product-Infrastructure-Team-Backlog-Deprecated board.Feb 25 2019, 12:41 PM
MSantos added a project: Continuous-Integration-Config.Mar 6 2019, 5:13 PM
MSantos added a subscriber: hashar.

@hashar what do you think about my thoughts here.

• Jhernandez changed the task status from Open to Stalled.Mar 13 2019, 3:45 PM
• Jhernandez triaged this task as Medium priority.
• Jhernandez moved this task from Needs triage to Backlog on the Product-Infrastructure-Team-Backlog-Deprecated board.
• Nforcer7 removed projects: Continuous-Integration-Config, Patch-For-Review, Product-Infrastructure-Team-Backlog-Deprecated, Technical-Debt, Maps (Kartographer), JsonConfig.Aug 1 2019, 5:15 PM
• Nforcer7 removed subscribers: hashar, gerritbot, MSantos and 4 others.
Aklapper added subscribers: hashar, MSantos, debt and 3 others.Aug 1 2019, 5:46 PM
Aklapper changed the task status from Stalled to Open.Aug 1 2019, 5:49 PM
Aklapper added projects: Continuous-Integration-Config, Patch-For-Review, Product-Infrastructure-Team-Backlog-Deprecated, Technical-Debt, Maps (Kartographer), JsonConfig.
Aklapper changed the task status from Open to Stalled.Aug 1 2019, 5:51 PM
gerritbot added a comment.Mar 23 2020, 9:13 PM

Change 464827 had a related patch set uploaded (by Jforrester; owner: MSantos):
[integration/config@master] Zuul: Run all Kartographer tests alongside each patch to JsonConfig

https://gerrit.wikimedia.org/r/464827

gerritbot added a comment.Mar 23 2020, 9:43 PM

Change 464827 merged by jenkins-bot:
[integration/config@master] Zuul: Run all Kartographer tests alongside each patch to JsonConfig

https://gerrit.wikimedia.org/r/464827

Stashbot added a comment.Mar 23 2020, 9:44 PM

Mentioned in SAL (#wikimedia-releng) [2020-03-23T21:43:59Z] <James_F> Zuul: Run all Kartographer tests alongside each patch to JsonConfig T163274

MSantos changed the task status from Stalled to Open.Apr 1 2020, 3:15 PM
MSantos moved this task from Backlog to Icebox on the Product-Infrastructure-Team-Backlog-Deprecated board.Feb 25 2021, 4:02 PM
Pppery removed a project: Patch-For-Review.Apr 1 2023, 4:10 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL