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

⬆️ Pre-0.0.2 housekeeping #305

Merged
merged 11 commits into from Apr 20, 2024

Conversation

JMicheli
Copy link
Contributor

This pull request is meant to do some general project housekeeping tasks.

It:

  • Bumps various crate and node package versions that could be increased with only minor refactoring;
  • Addresses a todo by refactoring the list_directory function;
  • Addresses a todo by adding removed thumbnail counting;
  • Addresses a todo by providing a description for ThumbnailGenerationJob; and
  • Addresses a todo by making PauseJob and ResumeJob take an AcknowledgeableCommand parameter.

@JMicheli JMicheli changed the title Pre-0.20 housekeeping Pre-0.0.2 housekeeping Apr 15, 2024
Copy link
Collaborator

@aaronleopold aaronleopold left a comment

Choose a reason for hiding this comment

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

This was a huge effort, I know managing dependency upgrades can be a bit tedious so I really appreciate you doing this!

I had a couple of nit picks of little importance, the one thing keeping me from approving and merging is that I think we need to bump the minimum rust version with these changes. Using the repo's configured 1.72.1 toolchain locally reported the following:

  • jsonwebtoken requires 1.73.0
  • clap_builder requires 1.74.0

apps/server/src/routers/api/v1/filesystem.rs Outdated Show resolved Hide resolved
Comment on lines +128 to +138
fn filter_if_hidden(entry: DirEntry) -> Option<DirEntry> {
let path = entry.path();
let stem = path.file_stem().unwrap_or_default();

// Remove hidden files starting with period
if stem.to_str().unwrap_or_default().starts_with('.') {
return None;
}

Some(entry)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not important for this PR, but might be worth exporting PathUtils trait from stump_core so we don't maintain duplicate logic, e.g. PathUtils::is_hidden_file.

core/src/filesystem/directory_listing.rs Outdated Show resolved Hide resolved
@JMicheli
Copy link
Contributor Author

Any reason not to bump it up to 1.77.2 (the current stable release)?

@aaronleopold
Copy link
Collaborator

Any reason not to bump it up to 1.77.2 (the current stable release)?

I don't think so, might as well give it a shot and see what CI says

@aaronleopold
Copy link
Collaborator

@JMicheli I updated the toolchain and a few other miscellaneous things (e.g. pdfium version, lints after updates, merge conflicts, etc).

There was actually a clippy bug I found, too!

Let me know if there's anything you want to add to this PR. I'll plan to merge this tomorrow after work, get a nightly image out, then 0.0.2 over the weekend. Thanks again for doing all of this cleanup!

@JMicheli
Copy link
Contributor Author

I won't have time today to do anything further, I say merge it if you're good to do so.

@aaronleopold aaronleopold changed the title Pre-0.0.2 housekeeping ⬆️ Pre-0.0.2 housekeeping Apr 20, 2024
@aaronleopold aaronleopold merged commit 724f7e0 into stumpapp:develop Apr 20, 2024
3 checks passed
@aaronleopold aaronleopold mentioned this pull request Apr 20, 2024
@JMicheli JMicheli deleted the 0.2.0-improvements branch May 1, 2024 20:08
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

2 participants