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

chore: simple cleanup #6173

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

ProteanCoding
Copy link

What does this PR change?

  • Removing dead code from a duplicated library import
  • Cleanup of configuration files

@ProteanCoding ProteanCoding changed the title Chore/cleanup code simple Simple cleanup Apr 18, 2024
@aixaCode
Copy link
Collaborator

Hi, thanks for this contribution, this is really nice clean up. We will check it, as we need to run some checks first.

@aixaCode aixaCode changed the title Simple cleanup chore: simple cleanup Apr 18, 2024
@ProteanCoding ProteanCoding marked this pull request as draft April 18, 2024 17:56
@ProteanCoding
Copy link
Author

Obviously this cleanup is not that simple...
The code duplication is certainly an issue, if the code that is used in circleci is not the code that is actively maintained.
Perhaps the live Plugins folder should be moved out of the Assets folder ?

@ProteanCoding ProteanCoding marked this pull request as ready for review April 18, 2024 18:16
@ProteanCoding
Copy link
Author

ProteanCoding commented Apr 18, 2024

Moved this code-duplication removal to its own PR, in the meantime, it should be possible to merge these remaining two simple cleanups.

@aixaCode
Copy link
Collaborator

aixaCode commented Apr 19, 2024

Could you describe why you are making this changes?

Could you follow this guide https://github.com/decentraland/unity-renderer/blob/dev/.github/CONTRIBUTING.md

@aixaCode aixaCode requested review from aixaCode and removed request for aixaCode April 19, 2024 15:32
@ProteanCoding
Copy link
Author

I do believe I have been following the CONTRIBUTING.md guide.
I will add comments to the two changes describing their purpose.

@@ -1,7 +1,7 @@
###############################################################################
# Set default behavior to automatically normalize line endings.
###############################################################################
* text=auto
* text=auto eol=lf
Copy link
Author

@ProteanCoding ProteanCoding Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On all modern OSes, modern editors (including VS Code) can deal with lf endings just fine, crlf is not needed.
If git is allowed to set the line endings to crlf before commits, this leads to discrepancies that are annoyingly useless at best.

And who knows what at worse ! Some of these files end up being used in a Docker environment, which will be expecting lf endings.

@@ -82,6 +82,7 @@ test/**/*.js
test/**/*.js.map
dist
!packages/shared/world/runtime-7/sourcemap/[email protected]
/protocol-temp/
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This folder can remain if the build process is interrupted before the end.
Its brief appearance has also been confusing my own file monitoring process.

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