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

11 configure cicd #13

Merged
merged 84 commits into from
Dec 20, 2023
Merged

11 configure cicd #13

merged 84 commits into from
Dec 20, 2023

Conversation

archeoss
Copy link
Contributor

@archeoss archeoss commented Sep 4, 2023

Resolves #11

Merging #9, #10 and #12 will make this PR easier to review.

@archeoss archeoss linked an issue Sep 4, 2023 that may be closed by this pull request
@archeoss archeoss force-pushed the 11-configure-cicd branch 2 times, most recently from 508ffd6 to d042ef7 Compare September 7, 2023 09:44
.github/workflows/rust.yml Outdated Show resolved Hide resolved
.github/workflows/rust.yml Outdated Show resolved Hide resolved
.github/workflows/rust.yml Outdated Show resolved Hide resolved
@archeoss
Copy link
Contributor Author

I think it's better to squash all those awkward commits on merge....

@ikopylov
Copy link
Member

Looks good, but should be rechecked when all previous requests will be merged

@archeoss archeoss changed the base branch from main to 5-create-dockerfile-and-docker-compose-to-simplify-deployment October 2, 2023 23:30
@archeoss
Copy link
Contributor Author

archeoss commented Oct 2, 2023

@archeoss archeoss force-pushed the 5-create-dockerfile-and-docker-compose-to-simplify-deployment branch from 047ea2c to 78100ab Compare October 3, 2023 00:14
@archeoss archeoss force-pushed the 5-create-dockerfile-and-docker-compose-to-simplify-deployment branch from 78100ab to ba64c2f Compare October 3, 2023 00:17
@archeoss archeoss force-pushed the 11-configure-cicd branch 2 times, most recently from 3a16c21 to 21151d8 Compare October 3, 2023 00:19
@ikopylov
Copy link
Member

There are 3 warning during docker build. Example:

#26 53.19 warning: function `main` is never used
#26 53.20  --> cli/src/lib.rs:1:4
#26 53.20   |
#26 53.20 1 | fn main() {println!("if you see this, the build broke")}
#26 53.20   |    ^^^^
#26 53.20   |
#26 53.20   = note: `#[warn(dead_code)]` on by default

Looks like lib.rs file was not copied before the actual build phase. Please, find out the reason

@archeoss
Copy link
Contributor Author

archeoss commented Nov 21, 2023

There are 3 warning during docker build. Example:

#26 53.19 warning: function `main` is never used
#26 53.20  --> cli/src/lib.rs:1:4
#26 53.20   |
#26 53.20 1 | fn main() {println!("if you see this, the build broke")}
#26 53.20   |    ^^^^
#26 53.20   |
#26 53.20   = note: `#[warn(dead_code)]` on by default

Looks like lib.rs file was not copied before the actual build phase. Please, find out the reason

I think 2 out of 3 warnings are part of the expected behavior, since the warnings appear before the project files are copied.

image

Or at least, this is the same behaviour that in the main bob repo....
image
(better message to avoid confusion perhaps?...)

The 3rd warning was about some dead code in release build (fixed)

@ikopylov
Copy link
Member

Ok, didn't notice, that the error is on the first phase. Then it is better to make lib.rs files empty on the first phase, because they don't need main function inside. Here (https://github.com/qoollo/bob-management/blob/main/dockerfiles/alpine/Dockerfile#L36) and here (https://github.com/qoollo/bob-management/blob/main/dockerfiles/alpine/Dockerfile#L39) you can write, for example, just a comment: echo "// if you see this, the build broke" > backend/src/lib.rs

@ikopylov
Copy link
Member

I think it would be good to add linter checks for frontend. This can be done by running yarn lint in frontend directory

.github/workflows/rust.yml Outdated Show resolved Hide resolved
.github/workflows/rust.yml Outdated Show resolved Hide resolved
@ikopylov ikopylov merged commit c2b74ef into main Dec 20, 2023
13 checks passed
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.

Configure CI/CD
2 participants