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

Created the linux module to improve the code structure #130

Merged
merged 36 commits into from
May 6, 2023

Conversation

0x61nas
Copy link
Contributor

@0x61nas 0x61nas commented Jan 12, 2023

Ceate a separate linux module and separate the common functions into one separated file

This PR is uncompleted yet, but I opened it to track the progress and to discussion.

I had to implement the Default trait for the Settings and the PackageSettings and the BundleSettings structs to create tests, but I'm not sure if this is the beast way or there are the best way to make this without implement the Default trait?

TODO:

  • Create a linux module and a common file for Linux
  • move the deb_bundle into linux module
  • move the rpm_bundle into linux module
  • implement the Default traite for the Settings and the PackageSettings and the BundleSettings structs to create tests
  • Create a test for the generate_desktop_file function
  • Create a test for the tar_and_gzip_dir function, and refactor it if need it
  • Create a test for the create_tar_from_dir function, and refactor it if need it (covered with the tar_and_gzip_dir function test)
  • Create a test for the create_file_with_data function, and refactor it if need it
  • Create a test for the total_dir_size function, and refactor it if need it
  • Refactor the generate_icon_files function, and create unit-test for it
  • Separate the generate_md5sum function from the deb_bundle.rs file to the common.rs file, and create a test for it

@0x61nas 0x61nas marked this pull request as draft January 12, 2023 11:39
0x61nas and others added 11 commits January 13, 2023 05:11
…in the `create_file_with_data` function test
I separated the functionality of `generate_icon_files` into 2 functions,
One takes care of png icon files: `generate_icon_files_png` and the
other takes cares of the rest: `generate_icon_files_non_png.

I also extracted out `get_dest_path`
@SergioRibera
Copy link

@anas-elgarhy do you need help? I just discovered this project and was thinking that it would be a good idea to accommodate it a little better in order to be able to scale on the other bundle systems out there and I see that you are already working on it

@0x61nas
Copy link
Contributor Author

0x61nas commented Mar 5, 2023

Yes @SergioRibera, I'm not available now to work on code unfortunately, so I really appreciate if you can help me, and review this pr anaselgarhy#1 or if u fix the conflicts

And thanks in advance.

@0x61nas
Copy link
Contributor Author

0x61nas commented Mar 12, 2023

I had to remove the implementation of the Default trait for the Settings and the BundleSettings structs in older to merge the base, because @slonopotamus removed the PackageSettings struct in #138 and used the Package struct from the cargo_metadata crate which is not implementing the Default trait and I think is not ideal if I implemented it.

But I still need an easy way to create an instance from the Settings struct in a some tests. So maybe I go to use the test-context crate, or I'll break down the functions and methods that's needing the Settings struct into small inline functions/methods, and test them.

@SergioRibera
Copy link

I had to remove the implementation of the trait for the and the structs in older to merge the base, because @slonopotamus removed the struct in #138 and used the struct from the crate which is not implementing the trait and I think is not ideal if I implemented it.Default``Settings``BundleSettings``PackageSettings``Package``cargo_metadata``Default

But I still need an easy way to create an instance from the struct in a some tests. So maybe I go to use the crate, or I'll break down the functions and methods that's needing the struct into small inline functions/methods, and test them.Settings``test-context``Settings

I think I can help you here too, let me check out that pr and see if I can add some details

@0x61nas
Copy link
Contributor Author

0x61nas commented Mar 12, 2023

I think I can help you here too, let me check out that pr and see if I can add some details

Nice, u can also take a look at the test_generate_desktop_file test, because this is the only place I use the default in.

I wonder if this test is not quite important, and we can just remove it

@0x61nas
Copy link
Contributor Author

0x61nas commented Mar 16, 2023

I think it's a good idea to just ignore test generate_desktop_file function for now

@0x61nas 0x61nas marked this pull request as ready for review March 16, 2023 11:32
@0x61nas
Copy link
Contributor Author

0x61nas commented Mar 16, 2023

The test action has a bug, It's using the stable rust, while we use nightly features
image

@0x61nas
Copy link
Contributor Author

0x61nas commented Mar 16, 2023

I can replace the assert_matches with the normal assert macro instead

@0x61nas 0x61nas marked this pull request as draft March 16, 2023 12:05
@0x61nas 0x61nas marked this pull request as ready for review March 16, 2023 14:48
@0x61nas
Copy link
Contributor Author

0x61nas commented Apr 28, 2023

@mdsteele Please don't forget to review this sad PR when you have a time

@0x61nas
Copy link
Contributor Author

0x61nas commented May 6, 2023

image

@mdsteele mdsteele merged commit 8e0db03 into burtonageo:master May 6, 2023
@0x61nas 0x61nas deleted the create-linux-module branch May 7, 2023 05:45
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.

Project structure issue
4 participants