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

✅ restore testing, add CI workflow and README.md #16

Merged
merged 3 commits into from
Jun 6, 2023

Conversation

AucaCoyan
Copy link
Contributor

Hi!
I pushed a bit more the repo and I think it's ready to move to the nushell org. Maybe nushell/nufmt?

Some comments:

  • I splitted the clippy_check.yml into pul_request.yml and main.yml, so we can have different workflows on every event.
  • I deleted everything inside the examples/ folder
  • I deleted the previous functions format_nu and format_nu_buffered and some tests with it. So this repo doesn't have more lines that are deprecated.
  • Also, I removed the benchmark and its dependencies.
  • and did some cleaning and rework on the README to make it fit the organization 😄

@AucaCoyan AucaCoyan mentioned this pull request Jun 5, 2023
22 tasks
@fdncred
Copy link
Collaborator

fdncred commented Jun 6, 2023

maybe we should leave the examples? i put them there are test cases. We'll also want to have benchmarks eventually. I mean we can delete it now but we'll need to bring it back at some point.

@AucaCoyan
Copy link
Contributor Author

Mmm I'm not sure. What do you think if we do examples in markdown documentation?

For bench marking, initially I was thinking on doing multi threading, but after looking on other big projects I realized they don't go that deep. I assume it's because it's a fast process in general.
On top of that, performance in rust is rarely an issue.

If we start waiting more that 2 seconds to format a file, then yes, we should lookout for performance. But initially it's not among my concerns.
I've never written performant software, or ever have to worry about memory management even, so bear in mind I'm not an expert

@fdncred
Copy link
Collaborator

fdncred commented Jun 6, 2023

i'm fine with having markdown docs but what i'm saying was that in order to test whether the software is formatting things properly, i had examples to test on.

as far as performance, it all depends on the size of the file. i have a > 4000 line script that takes a while to parse. so if parsing takes a while, reformatting could take longer.

i'm also fine with having lots of smaller tests to test in small bits.

@AucaCoyan
Copy link
Contributor Author

Sounds good to me.
I copied the example.nu which has 2400 lines and also ajust the bench for the current format_single_file function

How about now? should I add more changes? 🙌🏼

@fdncred
Copy link
Collaborator

fdncred commented Jun 6, 2023

We can change it later when we get around to it, but example.nu is not a valid script. It's just a dump of all the examples from nushell. So, the syntax isn't correct in it. Although, I think it's a good test to see what nufmt does with an invalid script.

@fdncred fdncred merged commit 2435913 into nushell:main Jun 6, 2023
4 checks passed
@AucaCoyan AucaCoyan mentioned this pull request Jun 6, 2023
@amtoine
Copy link
Member

amtoine commented Jun 6, 2023

@AucaCoyan
i do not see tests in there, am i missing something? 😮

@AucaCoyan
Copy link
Contributor Author

Mayhaps. The tests are below lib.rs and main.rs, to test both you can clone the project and run cargo test --workspace.
The workflows are in main.yml

@amtoine
Copy link
Member

amtoine commented Jun 7, 2023

Mayhaps. The tests are below lib.rs and main.rs, to test both you can clone the project and run cargo test --workspace.

ohh wow i just did not see them, my bad 😆

The workflows are in main.yml

👌

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

3 participants