Skip to content
This repository was archived by the owner on Oct 27, 2025. It is now read-only.

Add Lyon Cluster Map #640

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add Lyon Cluster Map #640

wants to merge 6 commits into from

Conversation

@gd-harco
Copy link

@gd-harco gd-harco commented Oct 10, 2024 •
edited by 42atomys
Loading

Describe the pull request
Added the Lyon cluster map in web/ui/src/lib/ClustersMaps.
Not sure about the REGEX part, I'm pretty bad with these.

Checklist

  • I have made the modifications or added tests related to my PR
  • I have run the tests and linters locally and they pass
  • I have added/updated the documentation for my RP

Additional context

Couldn't test it du to the issue with dev containers, but should work as intended.

@gd-harco gd-harco requested a review from 42atomys as a code owner October 10, 2024 11:13
@gd-harco gd-harco mentioned this pull request Oct 10, 2024
Open
1 task
42atomys
42atomys reviewed Oct 10, 2024
View reviewed changes
Copy link
Owner

@42atomys 42atomys left a comment

Choose a reason for hiding this comment

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

Hi, thanks for open your Pull request, I do an explicative review for the regexp and allow the run of CI to ensure all are good !

42atomys
42atomys approved these changes Oct 10, 2024
View reviewed changes
Copy link
Owner

@42atomys 42atomys left a comment

Choose a reason for hiding this comment

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

Linters and Test are green ✅

Congrats to do it without linters 🎉

@42atomys 42atomys linked an issue Oct 10, 2024 that may be closed by this pull request
feat: integrate lyon campus #319
Open
1 task
@gd-harco
Copy link
Author

gd-harco commented Oct 10, 2024

Thanks for the explanation !
I manage to use the linter in the dev container, so I don't have much merit 😅
Please feel free to @ me if some things need changes.

We will be receiving new computers in the month, so I'll be sure to create a new PR to update the map !

42atomys
42atomys approved these changes Oct 12, 2024
View reviewed changes
Copy link
Owner

@42atomys 42atomys left a comment

Choose a reason for hiding this comment

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

All seams to be good LGTM 🎉✅

I will send a mail to Lyon campus staff to allow usage of the building map and the PR will be merged at this time 💜

@42atomys 42atomys added type/improvement ✨ Improvement to an existing feature good first issue Good for newcomers state/needs information 🚧 We needs more information to go forwards priority/medium 🟨 Priority 3 - Not blocking but should be fixed soon aspect/interface 🕹 Concerns end-users' experience with the software domain/obvious 🟩 Represents the "known knowns" issue. It's Obviously labels Oct 12, 2024
@gd-harco
Copy link
Author

gd-harco commented Jan 10, 2025

The cluster has been updated with some new computer. I'll update the map as soon as possible. I suppose you still hadn't heard back from the 42 Lyon staff ?

@42atomys
Copy link
Owner

42atomys commented Jan 10, 2025

I suppose you still hadn't heard back from the 42 Lyon staff ?

You suppose well :( No response from lyon staff 😭

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

Reviewers

1 more reviewer

@42atomys 42atomys 42atomys approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

aspect/interface 🕹 Concerns end-users' experience with the software domain/obvious 🟩 Represents the "known knowns" issue. It's Obviously good first issue Good for newcomers priority/medium 🟨 Priority 3 - Not blocking but should be fixed soon size/L state/needs information 🚧 We needs more information to go forwards type/improvement ✨ Improvement to an existing feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: integrate lyon campus

2 participants

@gd-harco @42atomys