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

Unversion autogenerated files #6539

Conversation

GeniaHarrietBoeing
Copy link

I was working on a pattern and noticed that I had to add some autogenerated files to my commits, which I added to .gitignore.
There might be a reason to version these that I am not aware of.

Copy link

boring-cyborg bot commented Apr 10, 2024

Thanks for opening this pull request 😀 Please take a moment to read our contributing guidelines.

Copy link

vercel bot commented Apr 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
freesewing-dev ⬜️ Ignored (Inspect) Visit Preview Apr 10, 2024 1:53pm

Copy link

vercel bot commented Apr 10, 2024

Someone is attempting to deploy a commit to the freesewing Team on Vercel.

A member of the Team first needs to authorize it.

@joostdecock
Copy link
Member

The reason we added these originally was because we wanted to make it easier for people to work with our monorepo.

However, as the amount of files that are generated in the (pre)build step grows, it tends to become a bit unwieldy, so I think we should indeed re-visit this decision.

That being said, we need to be somewhat careful to ensure we don't break anything. Both the website and the stand-alone development environment will load (some) files directly from GitHub so we need to see that they are still there.

I'm going to add this to my todo list to look into this. I'll keep this open for now.

@vladh
Copy link

vladh commented Apr 10, 2024

Hmm, is there a use case where these files wouldn't be generated anyway? For example, for me, these files seem to be generated by yarn lab. Presumably other kinds of builds could also generate them as needed. Just trying to figure out in which scenario one would need to have these files prebuilt, if any.

@HaasJona
Copy link
Contributor

HaasJona commented Apr 11, 2024

Just trying to figure out in which scenario one would need to have these files prebuilt, if any.

Other than the lab environment, I think the main thing to check would be the new design environment from this tutorial: https://freesewing.dev/tutorials/pattern-design which is even more bare-bones than the lab (only contains designs inheriting the block bodies and an empty design).

@vladh
Copy link

vladh commented Apr 11, 2024

I think the main thing to check would be the new design environment

It looks like this PR poses no problems to the new-design environment, because this environment doesn't include the generated files this PR affects:

$ npx @freesewing/new-design
...
$ ls -la sites/lab/pages/new sites/lab/pages/account/patterns sites/shared/prebuild/data/
ls: cannot access 'sites/lab/pages/new': No such file or directory
ls: cannot access 'sites/lab/pages/account/patterns': No such file or directory
ls: cannot access 'sites/shared/prebuild/data/': No such file or directory

In fact, there seems to be no sites/ directory at all:

$ ls -la sites
ls: cannot access 'sites': No such file or directory

(I did run into #6543, but I think that is unrelated.)

Is there something else that should be checked?

@joostdecock
Copy link
Member

To be clear, the stand-along development environment (what you get when your run npx @freesewing/new-design is different from our monorepo. But it uses code from our monorepo, obviously.

Some of the things it downloads are prebuild artifacts that are now in the repo.
So if we drop those (somethink I and I think everyone is in favor of) we need to make sure we keep the stand-along environment from working, and deliver those files some other way. Either some sort of published archive, or an NPM package.

That's why it's going to take some time to address this.

@joostdecock
Copy link
Member

FYI: I have started working on #6552 and will probably tackle this within that scope.

@joostdecock
Copy link
Member

I'm going to close this in favor of #6552 (which is on our roadmap now)

@joostdecock joostdecock closed this May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants