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

Change races to activities #172

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Conversation

Mathieu414
Copy link
Collaborator

See the project item for the idea of the changes.

Quite a lot of things. Changed a bit how the data is displayed in the activities accordion, so that it becomes clearer what is the event description and what is your own registration.

@Atmos4
Copy link
Owner

Atmos4 commented Mar 25, 2024

@Mathieu414 you might want to rebase to master with this branch! You can also either worktree or stash our progress to make it easier to review, and create another work item!

@Mathieu414 Mathieu414 force-pushed the 171-change-races-into-activities branch from 7e48a37 to 1e88d3d Compare March 27, 2024 12:34
@Atmos4
Copy link
Owner

Atmos4 commented Apr 2, 2024

I don't like the current data migration:

  • it has var-dumps. Please if you want to display things, format them in a readable way
  • it does not work and throws errors. Clean it up before the merge
  • it is non revertible. Please implement a "revert" button, or better yet use the built in migrations from doctrine!

If you want to generate an empty migration file with Doctrine, you can use the following command:

php bin/doctrine migrations:generate

To be clear, the real blocker here is that it does not work. If it would work flawlessly I would probably approve. I however think this migration is dangerous, and would appreciate if you make the code cleaner and less risky to execute.

So since it just does not work, I would like if you would try to add a test for the migration! If you want some help creating a test I can of course help out :)

@Mathieu414 Mathieu414 linked an issue Apr 2, 2024 that may be closed by this pull request
@Mathieu414 Mathieu414 force-pushed the 171-change-races-into-activities branch 2 times, most recently from 75d775f to ebde724 Compare April 3, 2024 16:06
Mathieu414 and others added 2 commits April 8, 2024 10:54
- migrate visual and services
- add migration helper page
- add helpers and better structure to handle route protection
- improve rollback to prevent data corruption
- improve user experience
@Atmos4 Atmos4 force-pushed the 171-change-races-into-activities branch from 612347d to 144fa4f Compare April 8, 2024 08:55
@Atmos4 Atmos4 merged commit ffbd3ad into master Apr 8, 2024
1 check passed
@Atmos4 Atmos4 deleted the 171-change-races-into-activities branch April 8, 2024 14:16
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.

Change races into activities
2 participants