-
Notifications
You must be signed in to change notification settings - Fork 822
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
encoding/json: Add Custom JSON Package with Build Tag Support for Sonic #1623
base: master
Are you sure you want to change the base?
Changes from 6 commits
8333e5c
c47137c
6e24f60
b70b9e7
84dd409
711db77
223f786
9a72688
2f69362
79c85c9
b4f0243
417e0aa
83f1d57
f2d56cf
7bd7cc4
79ebfa1
6b6fb5e
ac47cc6
135e2b6
2d08549
1a82764
1ca8eaa
ab5fd01
1697977
3905b62
1c9f9c2
4f8e487
9cffc8f
1b15e4c
a7ac15d
f23f3c3
fba5316
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,22 +13,52 @@ jobs: | |
goarch: amd64 | ||
psql: true | ||
skip_wrapper_tests: false | ||
sonic: false | ||
- os: ubuntu-latest | ||
goarch: 386 | ||
psql: true | ||
skip_wrapper_tests: true | ||
sonic: false | ||
- os: macos-latest | ||
goarch: amd64 | ||
psql: true | ||
skip_wrapper_tests: true | ||
sonic: false | ||
- os: macos-13 # beta | ||
goarch: amd64 | ||
psql: true | ||
skip_wrapper_tests: true | ||
sonic: false | ||
- os: windows-latest | ||
goarch: amd64 | ||
psql: true | ||
skip_wrapper_tests: true | ||
sonic: false | ||
- os: ubuntu-latest-sonic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you should stick to the same style as the existing builds names with the Also, do we need two of each build? Would one extra one with sonic enabled suffice? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That should have been reverted. I think.
@thrasher- Confirming entire suite test or can just have one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can target amd64 linux for now since that's our main deployment OS in use ATM There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
goarch: amd64 | ||
psql: true | ||
skip_wrapper_tests: false | ||
sonic: true | ||
- os: ubuntu-latest-sonic | ||
goarch: 386 | ||
psql: true | ||
skip_wrapper_tests: true | ||
sonic: true | ||
- os: macos-latest-sonic | ||
goarch: amd64 | ||
psql: true | ||
skip_wrapper_tests: true | ||
sonic: true | ||
- os: macos-13-sonic # beta | ||
goarch: amd64 | ||
psql: true | ||
skip_wrapper_tests: true | ||
sonic: true | ||
- os: windows-latest-sonic | ||
goarch: amd64 | ||
psql: true | ||
skip_wrapper_tests: true | ||
sonic: true | ||
|
||
runs-on: ${{ matrix.os }} | ||
|
||
|
@@ -84,13 +114,21 @@ jobs: | |
echo "CGO_ENABLED=1" >> $GITHUB_ENV | ||
shell: bash | ||
|
||
- name: Set test flags | ||
run: | | ||
TEST_FLAGS="" | ||
if [ "${{ matrix.sonic }}" = "true" ]; then | ||
TEST_FLAGS="-tags sonic" | ||
fi | ||
shell: bash | ||
|
||
- name: Test | ||
run: | # PGSERVICEFILE isn't supported by lib/pq and will cause a panic if set | ||
unset PGSERVICEFILE | ||
if [ "${{ matrix.goarch }}" = "386" ]; then | ||
go test -coverprofile coverage.txt -covermode atomic ./... | ||
go test $TEST_FLAGS -coverprofile coverage.txt -covermode atomic ./... | ||
else | ||
go test -race -coverprofile coverage.txt -covermode atomic ./... | ||
go test $TEST_FLAGS -race -coverprofile coverage.txt -covermode atomic ./... | ||
fi | ||
shell: bash | ||
env: | ||
|
@@ -169,4 +207,4 @@ jobs: | |
cd web/ | ||
npm install | ||
npm run lint | ||
npm run build | ||
npm run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this line, would you consider a log on GCT start to say which one is enabled? Yes, you have build tags, but then it puts it in the logs of the application
Could add to the build tag files to call like
json.Version()
where itsfmt.Println("hello my name is Sonic and I gotta go fast, okay?")
as an exampleThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs checking