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.

Sign up for GitHub

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

Jump to bottom

Stop support for linux/arm(32bit) #431

Merged
merged 2 commits into from
Jun 13, 2022
Merged

Stop support for linux/arm(32bit) #431

merged 2 commits into from
Jun 13, 2022

Conversation

da1suk8
Copy link
Member

@da1suk8 da1suk8 commented Jun 10, 2022 •
edited
Loading

Description

This PR stop support for linux/arm(32bit) because linux/arm(32bit) is for smart devices(mobile phone etc ..), I thought it could be removed for maintenance costs.
Continued support for linux/amd64 and linux/arm64.

This PR contains the following changes

  • remove arm(32bit) test in .github/workflow/build.yml

@Kynea0b Kynea0b added the C: enhancement Classification: New feature or its request, or improvement in maintainability of code label Jun 10, 2022
tnasu
tnasu previously approved these changes Jun 10, 2022
View reviewed changes
Copy link
Member

@tnasu tnasu left a comment

Choose a reason for hiding this comment

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

I agree with you

@torao torao requested review from tnasu and torao June 10, 2022 08:43
Kynea0b
Kynea0b previously approved these changes Jun 10, 2022
View reviewed changes
torao
torao suggested changes Jun 10, 2022
View reviewed changes
Copy link
Contributor

@torao torao left a comment

Choose a reason for hiding this comment

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

I've checked other codes and documents that reference arm (32-bit).

The only written mention of ARM support was in CHANGELOG, which wouldn't need to be corrected (need to mention for 32-bit ARM in the next release).

About build, the following one may need to be changed at least (or the binary build will fail at release time).

  1. https://github.com/da1suk8/ostracon/blob/fix/remove_arm32/.goreleaser.yml#L20

The following are items that don't interfere with the development process, such as builds and releases, but that I feel should be removed to avoid confusion.

  1. https://github.com/da1suk8/ostracon/blob/fix/remove_arm32/crypto/vrf/internal/vrf/vrf.go#L15-L16
  2. https://github.com/da1suk8/ostracon/blob/fix/remove_arm32/scripts/dist.sh#L21-L23

Could you give us your opinion too, @tnasu -san?

@tnasu
Copy link
Member

tnasu commented Jun 13, 2022

#431 (review)

Yes, @da1suk8 should address them in this PR. Thanks.

@da1suk8 da1suk8 dismissed stale reviews from Kynea0b and tnasu via fcc4348 June 13, 2022 05:03
@codecov
Copy link

codecov bot commented Jun 13, 2022

Codecov Report

Merging #431 (fcc4348) into main (f19316f) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #431      +/-   ##
==========================================
- Coverage   65.28%   65.28%   -0.01%     
==========================================
  Files         277      277              
  Lines       37842    37842              
==========================================
- Hits        24707    24704       -3     
- Misses      11321    11323       +2     
- Partials     1814     1815       +1
Impacted Files Coverage Δ
crypto/bls/bls.go 45.45% <0.00%> (-2.80%) ⬇️
mempool/reactor.go 78.57% <0.00%> (-1.10%) ⬇️
p2p/conn/connection.go 79.80% <0.00%> (-0.58%) ⬇️
consensus/state.go 73.15% <0.00%> (-0.53%) ⬇️
p2p/pex/addrbook.go 71.01% <0.00%> (-0.51%) ⬇️
light/client.go 61.61% <0.00%> (-0.45%) ⬇️
proxy/multi_app_conn.go 47.66% <0.00%> (ø)
consensus/reactor.go 75.37% <0.00%> (+0.44%) ⬆️
blockchain/v0/pool.go 79.58% <0.00%> (+0.51%) ⬆️
p2p/pex/pex_reactor.go 79.67% <0.00%> (+0.61%) ⬆️
... and 3 more

@da1suk8 da1suk8 requested a review from torao June 13, 2022 05:45
tnasu
tnasu approved these changes Jun 13, 2022
View reviewed changes
Copy link
Member

@tnasu tnasu left a comment

Choose a reason for hiding this comment

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

LGTM again. Thanks for your contribution.

@torao
Copy link
Contributor

torao commented Jun 13, 2022 •
edited
Loading

I feel this title contains a bit of ambiguity. Because guessing that this omitted subject, one can be read as "(current Ostracon) doesn't support ARM 32-bit", which can be interpreted as if this PR fixes to support ARM 32-bit.

Could you please change the title to something clearer like "Stop ... support", or "Stop support for ..."?

@da1suk8 da1suk8 changed the title doesn't support linux/arm(32bit) Stop support for linux/arm(32bit) Jun 13, 2022
@da1suk8 da1suk8 merged commit 382a443 into Finschia:main Jun 13, 2022
@da1suk8 da1suk8 deleted the fix/remove_arm32 branch June 13, 2022 10:20
zemyblue added a commit to zemyblue/finschia-sdk that referenced this pull request Jun 16, 2022
@zemyblue
ci: stop support for linux/arm(32bit) according to Finschia/ostracon#431
420e4b5 


Signed-off-by: zemyblue <zemyblue@gmail.com>
@zemyblue zemyblue mentioned this pull request Jun 16, 2022
Merged
5 tasks
zemyblue added a commit to Finschia/finschia-sdk that referenced this pull request Jun 17, 2022
@zemyblue
feat: change gRPC path to original cosmos path (#564)
c1a1267 
* feat: change protobuf path to original cosmos path.

Signed-off-by: zemyblue <zemyblue@gmail.com>

* fix: fix multisig gRPC package path error and remove unused import in gRPC define.

Signed-off-by: zemyblue <zemyblue@gmail.com>

* ci: stop support for linux/arm(32bit) according to Finschia/ostracon#431

Signed-off-by: zemyblue <zemyblue@gmail.com>

* fix: unittest error of `TestQueryABCIHeight`

Signed-off-by: zemyblue <zemyblue@gmail.com>

* fix: lint error of `foundation` module

Signed-off-by: zemyblue <zemyblue@gmail.com>

* fix: unittest error

Signed-off-by: zemyblue <zemyblue@gmail.com>

* doc: update changelog

Signed-off-by: zemyblue <zemyblue@gmail.com>

* chore: rollback the data race problem.

Signed-off-by: zemyblue <zemyblue@gmail.com>

* doc: update changelog

Signed-off-by: zemyblue <zemyblue@gmail.com>

* Revert "doc: update changelog"

This reverts commit 0da197d.
tnasu added a commit to tnasu/ostracon that referenced this pull request Jun 17, 2022
@tnasu
(fixup) Move Finschia#431 into BREAKING CHANGE
c385b67
tnasu added a commit that referenced this pull request Jun 20, 2022
@tnasu
tag v1.0.6 (#433)
f33c191 
* tag v1.0.6

* (fixup) Move #431 into BREAKING CHANGE
@tnasu tnasu mentioned this pull request Jun 20, 2022
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@torao torao torao approved these changes

@tnasu tnasu tnasu approved these changes

@Kynea0b Kynea0b Kynea0b left review comments

Assignees

@da1suk8 da1suk8

Labels
C: enhancement Classification: New feature or its request, or improvement in maintainability of code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@da1suk8 @tnasu @torao @Kynea0b