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

First release #1

Merged
merged 7 commits into from
Oct 28, 2019
Merged

First release #1

merged 7 commits into from
Oct 28, 2019

Conversation

ljmf00
Copy link
Contributor

@ljmf00 ljmf00 commented Sep 20, 2019

No description provided.

@ljmf00 ljmf00 self-assigned this Sep 20, 2019
@ljmf00 ljmf00 force-pushed the luis-ferreira/first-release branch 10 times, most recently from 9102cc0 to cfe0b1a Compare September 24, 2019 19:07
@ljmf00 ljmf00 marked this pull request as ready for review September 24, 2019 20:02
@ljmf00 ljmf00 force-pushed the luis-ferreira/first-release branch 4 times, most recently from 681f896 to f92c685 Compare September 27, 2019 16:44
@ljmf00 ljmf00 force-pushed the luis-ferreira/first-release branch from f92c685 to eeda03e Compare October 2, 2019 18:42
@ljmf00 ljmf00 force-pushed the luis-ferreira/first-release branch from eeda03e to 509af18 Compare October 8, 2019 18:01
@ljmf00 ljmf00 requested a review from mrfyda October 9, 2019 17:57
@ljmf00 ljmf00 force-pushed the luis-ferreira/first-release branch 2 times, most recently from ed27bfd to 2c92fa0 Compare October 15, 2019 14:39
@ljmf00 ljmf00 requested a review from gjsduarte October 15, 2019 16:14
Copy link

@andreaTP andreaTP left a comment

Choose a reason for hiding this comment

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

the review is not finished, but here you have a start.

.gitmodules Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
docs/description/description.json Outdated Show resolved Hide resolved
docs/patterns.json Outdated Show resolved Hide resolved
src/Analyzer/CodeAnalyzer.cs Show resolved Hide resolved
src/Analyzer/CodeAnalyzer.cs Outdated Show resolved Hide resolved
src/Analyzer/CodeAnalyzer.cs Outdated Show resolved Hide resolved
src/Analyzer/CodeAnalyzer.cs Outdated Show resolved Hide resolved
src/Analyzer/CodeAnalyzer.cs Show resolved Hide resolved
src/DocsGenerator/Helpers/CategoryHelper.cs Outdated Show resolved Hide resolved
@ljmf00 ljmf00 force-pushed the luis-ferreira/first-release branch 3 times, most recently from fc8fd47 to 66cca3c Compare October 22, 2019 16:00
.circleci/config.yml Outdated Show resolved Hide resolved
.tsqllint-version Outdated Show resolved Hide resolved
Dockerfile Outdated
@@ -0,0 +1,10 @@
FROM mcr.microsoft.com/dotnet/core/runtime:2.2-alpine

COPY src/Analyzer/bin/Release/netcoreapp2.2/publish/*.dll /opt/docker/bin/

Choose a reason for hiding this comment

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

this is dependent from operations done on the host machine IIUC, can we use multi-stage build here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this just depends on the framework target, and here, we just use one framework target, which is netcoreapp2.2. The .dll file is CIL bytecode, so it's host independent. You just need to make sure you have the same runtime installed.

Choose a reason for hiding this comment

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

this doesn't come with the repository itself.
Should be done in a multi-stage IMHO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

scripts/bootstrap.sh Outdated Show resolved Hide resolved
scripts/bootstrap.sh Show resolved Hide resolved
src/Analyzer/CodeAnalyzer.cs Show resolved Hide resolved
src/DocsGenerator/Helpers/CategoryHelper.cs Show resolved Hide resolved
src/DocsGenerator/Helpers/LevelHelper.cs Show resolved Hide resolved
src/DocsGenerator/Program.cs Outdated Show resolved Hide resolved
andreaTP
andreaTP previously approved these changes Oct 25, 2019
description: "Whether to cache bootstrap files"
default: false
docker:
#TODO: switch to docker image

Choose a reason for hiding this comment

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

do we need to keep this TODO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

scripts/bootstrap.sh Show resolved Hide resolved
src/Analyzer/CodeAnalyzer.cs Show resolved Hide resolved
src/Analyzer/CodeAnalyzer.cs Show resolved Hide resolved
Copy link

@andreaTP andreaTP left a comment

Choose a reason for hiding this comment

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

last minor comment and LGTM

Dockerfile Outdated
@@ -0,0 +1,18 @@
FROM mcr.microsoft.com/dotnet/core/sdk:2.2

Choose a reason for hiding this comment

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

FROM mcr.microsoft.com/dotnet/core/sdk:2.2 AS buildimage

Dockerfile Outdated Show resolved Hide resolved
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
@ljmf00 ljmf00 force-pushed the luis-ferreira/first-release branch from 37c51db to b07bf86 Compare October 28, 2019 10:46
Copy link

@andreaTP andreaTP left a comment

Choose a reason for hiding this comment

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

please next time try to use the guidelines for the commit messages, but LGTM

@ljmf00 ljmf00 merged commit e05596b into master Oct 28, 2019
@delete-merged-branch delete-merged-branch bot deleted the luis-ferreira/first-release branch October 28, 2019 10:57
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