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

revert: re-enable workspaces #278

Closed
wants to merge 5 commits into from

Conversation

blaggacao
Copy link
Contributor

f9c8db3

The original "chore" commit didn't specify a specific reason.

On the other hand, the lack of a proper workspace configuration breaks the Frappix build of Gameplan.

This PR re-instates the workspaces and heals Frappix builds of Gameplan

@blaggacao blaggacao force-pushed the revert/re-enable-workspaces branch 2 times, most recently from f36cc1c to 9a7b551 Compare March 23, 2024 15:48
Copy link

cypress bot commented Mar 23, 2024

5 failed tests on run #248 ↗︎

5 1 0 0 Flakiness 0

Details:

Merge 41b5936 into d357258...
Project: gameplan Commit: 2b8d445c85 ℹ️
Status: Failed Duration: 02:40 💡
Started: Jul 23, 2024 1:43 AM Ended: Jul 23, 2024 1:46 AM
Failed  discussion.cy.js • 1 failed test

View Output Video

Test Artifacts
Discussion > all discussion actions Screenshots Video
Failed  onboarding.cy.js • 1 failed test

View Output Video

Test Artifacts
Onboarding > onboarding works Screenshots Video
Failed  project.cy.js • 1 failed test

View Output Video

Test Artifacts
Project > project creation, move to team and archive Screenshots Video
Failed  task.cy.js • 1 failed test

View Output Video

Test Artifacts
Task > task actions Screenshots Video
Failed  team.cy.js • 1 failed test

View Output Video

Test Artifacts
Team > team creation, readme edit and archive Screenshots Video

Review all test suite changes for PR #278 ↗︎

@blaggacao blaggacao force-pushed the revert/re-enable-workspaces branch from 9a7b551 to 511fe16 Compare May 30, 2024 14:15
blaggacao added a commit to blaggacao/frappix that referenced this pull request May 30, 2024
@blaggacao
Copy link
Contributor Author

@netchampfaris I removed the motion stuff. I guess this could now be re-considered? It would really help me to package Gameplan in Frappix.

@blaggacao
Copy link
Contributor Author

@netchampfaris Could this be reconsidered? It's kind of a particular interest requirement, but given that the other new style apps use workspaces (afaik: Crm, Builder & Insights) there might be an independent value on aligning the code layout between projects.

As a collateral it would really help me package GamePlan properly for Frappix. 😄

@blaggacao
Copy link
Contributor Author

ping

@blaggacao blaggacao force-pushed the revert/re-enable-workspaces branch 2 times, most recently from 0ff6746 to 81649f6 Compare July 2, 2024 18:03
@blaggacao blaggacao force-pushed the revert/re-enable-workspaces branch from 81649f6 to 6411b36 Compare July 22, 2024 19:07
@blaggacao blaggacao force-pushed the revert/re-enable-workspaces branch from 6411b36 to 34d4b8c Compare July 23, 2024 00:27
@blaggacao
Copy link
Contributor Author

@netchampfaris Could you please verify if 41b5936 is also an independent fix? Thank you!

@netchampfaris
Copy link
Contributor

@blaggacao

Hey, workspaces is only meant for development.

I have no idea what is "Frappix", but Gameplan's frontend assets should build just fine without workspaces since it will download frappe-ui from npm based on the package.json.

@netchampfaris
Copy link
Contributor

@netchampfaris Could you please verify if 41b5936 is also an independent fix? Thank you!

Expand/collapse bug is fixed 632a355

@blaggacao
Copy link
Contributor Author

blaggacao commented Nov 28, 2024

Hey https://github.com/blaggacao/frappix uses a sandboxed build. I don't remember, what exactly failed, but if the build itself (not fetching pinned! resources) requires network access, then that's a big red flag, already.

Ever since it failed, I started carrying this patch. I'll have to find time to rebase and re-invigorate the claim.

The gameplan packaging recipe is here: https://github.com/blaggacao/frappix/blob/main/src%2Foverlays%2Ffrappe%2Fgameplan%2Fdefault.nix — need to check with the latest developments.

@blaggacao
Copy link
Contributor Author

Ah, I think I remember the problem:

This file is empty https://github.com/frappe/gameplan/blob/main/yarn.lock but at the same time this is the level there all other apps pin their dependencies.

I'd have to check if there's a way to package the individual components in isolation, but are they properly declaring dependencies between each other?

@netchampfaris
Copy link
Contributor

@blaggacao

This is the correct https://github.com/frappe/gameplan/blob/main/frontend/yarn.lock file with pinned dependencies.

@blaggacao blaggacao deleted the revert/re-enable-workspaces branch January 1, 2025 14:36
@blaggacao
Copy link
Contributor Author

Thanks, yea I'll try to make it work that way.

@blaggacao
Copy link
Contributor Author

Ok, I just hit this again, logged it for now: blaggacao/frappix#18 (comment)

If there's no way that regular workspaces may be used, I need to figure out another way or leave gameplan un-supported for the time being.

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.

2 participants