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

Small fixes #269

Closed
wants to merge 7 commits into from
Closed

Small fixes #269

wants to merge 7 commits into from

Conversation

OldiLo
Copy link
Contributor

@OldiLo OldiLo commented May 20, 2022

This have pr #268 and small fixes

@ZyanKLee
Copy link
Contributor

Please do not include other PRs with your small fixes.

@TheDukeofErl
Copy link
Collaborator

At some point a discussion needs to be had in greater depth over the removal of the vendor folder: #257 initially set out to do the same thing.

@ZyanKLee
Copy link
Contributor

At some point a discussion needs to be had in greater depth over the removal of the vendor folder: #257 initially set out to do the same thing.

Sure, but this is not "small fixes":

Screenshot_20220520-215649~2.png

@OldiLo
Copy link
Contributor Author

OldiLo commented May 20, 2022

Remove vendor folder, add to .gitignore .vscode .cache noisetorch Fix build with pr #268

@OldiLo
Copy link
Contributor Author

OldiLo commented May 20, 2022

And update go dependencies

@ZyanKLee
Copy link
Contributor

My suggestion to go forward is to incorporate your small fix to c/ladspa/Makefile into the PR #268, just to handle that change separately from the others.

Your suggestions to add those entries to .gitignore are a no-brainer to me - means: I would merge those separately without batting an eye.

The update to Go dependencies are a bigger issue for me. I will not do this right now; mostly because we did not decide on how to handle the vendor directory going forward. We should have a discussion on that topic. But to have that I would like to wait for a) the review being done and b) for lawl's return (if that happens within a considerable amount of time). He seemed to have a firm stance in those regards and I don't want to just brush that aside.

@OldiLo
Copy link
Contributor Author

OldiLo commented May 21, 2022

I think deleting the vendor folder is a good idea (all malicious code could be stored there). As for updating dependencies, I think it's also a good idea (there's only nucular, go-glfw, gioui) for bug fixes

@Dadadah
Copy link
Collaborator

Dadadah commented May 21, 2022

While I agree that deleting the vendor folder is a good idea (hell i made the pr) it's good practice to keep pr's focused on a single issue to make merging things more straight forward.

@OldiLo OldiLo closed this Jun 1, 2022
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.

None yet

4 participants