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

Add environmental variable to control path of media #683

Merged
merged 1 commit into from Oct 28, 2019

Conversation

@epsilon-phase
Copy link
Contributor

epsilon-phase commented Oct 24, 2019

This was a request made on the matrix chat that will make it possible to release a snap version, that environmental variable being MEDIA_UPLOAD_DIRECTORY

@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 24, 2019

Codecov Report

Merging #683 into master will decrease coverage by 0.03%.
The diff coverage is 21.05%.

@@            Coverage Diff            @@
##           master    #683      +/-   ##
=========================================
- Coverage   39.83%   39.8%   -0.04%     
=========================================
  Files          72      72              
  Lines        9368    9383      +15     
  Branches     2219    2219              
=========================================
+ Hits         3732    3735       +3     
- Misses       4580    4592      +12     
  Partials     1056    1056
@epsilon-phase epsilon-phase force-pushed the epsilon-phase:Media-path branch from a5e5530 to 5b56fc8 Oct 25, 2019
Copy link
Contributor

RAOF left a comment

This looks correct to me; I'm not familiar with Rocket's #[get()] routing stuff, so someone else should check that this is correct.

I'll give it a test! Thanks!

@epsilon-phase epsilon-phase force-pushed the epsilon-phase:Media-path branch from 5b56fc8 to 252ad9e Oct 27, 2019
Copy link
Member

igalic left a comment

👀

src/routes/medias.rs Show resolved Hide resolved
Copy link
Member

fdb-hiroshima left a comment

I haven't tested yet, but apart from a minor path mistake, it looks good to me 🙂

.join("media")
.join(format!("{}.{}", GUID::rand().to_string(), ext));
let path = Path::new(&super::CONFIG.media_directory)
.join("media")

This comment has been minimized.

Copy link
@fdb-hiroshima

fdb-hiroshima Oct 27, 2019

Member

the media part is already included in the default value of media_directory (static/media). I don't know if it should be removed from here or from there (it depends on how custom directory should appear), but I think there is one too much

This comment has been minimized.

Copy link
@epsilon-phase

epsilon-phase Oct 28, 2019

Author Contributor

Oh, thanks for the catch.

@epsilon-phase epsilon-phase force-pushed the epsilon-phase:Media-path branch from 252ad9e to 612e86a Oct 28, 2019
@epsilon-phase epsilon-phase force-pushed the epsilon-phase:Media-path branch from 612e86a to 56aee22 Oct 28, 2019
@RAOF

This comment has been minimized.

Copy link
Contributor

RAOF commented Oct 28, 2019

I've built a snap from this locally, setting the new MEDIA_UPLOAD_DIRECTORY appropriately, and it works!

Once this lands I'll push a PR for the snapcraft metadata and we'll have a fully-functional snap that I can then document! 😁

Copy link
Member

AnaGelez left a comment

Looks good to me! Thank you for your help! 👍

@AnaGelez AnaGelez merged commit 866465c into Plume-org:master Oct 28, 2019
10 checks passed
10 checks passed
build and test Workflow: build and test
Details
ci/circleci: cargo fmt Your tests passed on CircleCI!
Details
ci/circleci: clippy-1 Your tests passed on CircleCI!
Details
ci/circleci: clippy-2 Your tests passed on CircleCI!
Details
ci/circleci: integration-1 Your tests passed on CircleCI!
Details
ci/circleci: integration-2 Your tests passed on CircleCI!
Details
ci/circleci: release-1 Your tests passed on CircleCI!
Details
ci/circleci: release-2 Your tests passed on CircleCI!
Details
ci/circleci: unit-1 Your tests passed on CircleCI!
Details
ci/circleci: unit-2 Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.