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

Repo organization: move tests out of examples #5992

Closed
eladb opened this issue Mar 19, 2024 · 14 comments · Fixed by #7063
Closed

Repo organization: move tests out of examples #5992

eladb opened this issue Mar 19, 2024 · 14 comments · Fixed by #7063
Assignees
Labels
👷‍♀️ contributor-experience Improvements for contributors' experience refactor Stale

Comments

@eladb
Copy link
Contributor

eladb commented Mar 19, 2024

Currently all of our tests are under /examples/tests. That's a bit weird.

Can we just move them to /tests ?

@monadabot monadabot added this to Wing Mar 19, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New - not properly defined in Wing Mar 19, 2024
@Chriscbr
Copy link
Contributor

Adding on an idea - I think it would be nice if all packages (including TypeScript-based and Rust-based ones) were in the same directory, /packages, instead of split between /apps and /libs. (2 cents)

@Chriscbr Chriscbr added 👷‍♀️ contributor-experience Improvements for contributors' experience needs-discussion Further discussion is needed prior to impl labels Mar 19, 2024
@MarkMcCulloh
Copy link
Contributor

If we move the tests folder out, I would also just get rid of the examples folder. The "proposed" folder can just be deleted, and the "fixture" projects can either moved inside tests or just put alongside other projects.

all packages (including TypeScript-based and Rust-based ones) were in the same directory, /packages

I'm cool with this, If we combine everything we could also start respecting the convention of a hierarchy the matches the package name. i.e. wingsdk would be in /packages/@winglang/sdk

@Chriscbr
Copy link
Contributor

The "proposed" folder can just be deleted

Agree, I think we can gracefully archive them to git history (or keep them as a fun historical artifact somewhere on the contributing docsite)

@eladb
Copy link
Contributor Author

eladb commented Mar 19, 2024

+1 on /packages/@winglang/sdk, /test, /examples

@staycoolcall911 staycoolcall911 moved this from 🆕 New - not properly defined to 🤝 Backlog - handoff to owners in Wing Mar 20, 2024
@staycoolcall911
Copy link
Contributor

Summarizing the actions specified in this issue, for any happy contributor out there who wants to pick this up:

  1. Move all of our tests one dir up. From /examples/tests to /tests.
  2. Move the 3 fixture packages under /tests. From /examples/*-fixture to /tests/*-fixture.
  3. Delete /examples (the only thing left at this point would be /examples/proposed).
  4. Combine /apps and /libs to /packages.

@staycoolcall911 staycoolcall911 added the good first issue Good for newcomers label Mar 20, 2024
mergify bot pushed a commit that referenced this issue Apr 4, 2024
Archived to... git history.

Related to #5992

## Checklist

- [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [ ] Description explains motivation and solution
- [ ] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@govindplal
Copy link

Can I work on this?

@staycoolcall911
Copy link
Contributor

But of course @govindplal :)
Let us know if you have any questions, either here or in Slack #dev channel:
https://t.winglang.io/slack

@govindplal
Copy link

This is what I understood is to be done :
Move /tests outside /examples
Move /jsii-fixture, /ts-fixture, /js-fixture to /tests
Create a new /packages and move all the elements of /apps and /libs into /packages
Update necessary paths if required.
Delete /examples, /apps, /libs.

@staycoolcall911 Can someone confirm this

@staycoolcall911
Copy link
Contributor

Yes indeed @govindplal - you got it right

@tsuf239
Copy link
Contributor

tsuf239 commented May 5, 2024

also, please take a look at where we use examples/tests (or any other path) in the code, docs, gh workflows and configuration files, and package.json :)
I would advise to do it incrementally- moving out one or two directories in each PR

@govindplal
Copy link

also, please take a look at where we use examples/tests (or any other path) in the code, docs, gh workflows and configuration files, and package.json :) I would advise to do it incrementally- moving out one or two directories in each PR

Yeah, sure. Got held up with something else for the past days. I'm working on it!

@revitalbarletz
Copy link

Hi @govindplal I am trying to reach out through Discord - I could not see you there, do you want to join to we can continue the discussion there? https://t.winglang.io/discord

@staycoolcall911 staycoolcall911 removed good first issue Good for newcomers needs-discussion Further discussion is needed prior to impl labels Jun 3, 2024
Copy link

github-actions bot commented Sep 2, 2024

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@github-actions github-actions bot added the Stale label Sep 2, 2024
@github-project-automation github-project-automation bot moved this from 🤝 Backlog - handoff to owners to ✅ Done in Wing Sep 3, 2024
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.83.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👷‍♀️ contributor-experience Improvements for contributors' experience refactor Stale
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants