Skip to content
This repository was archived by the owner on Oct 2, 2020. It is now read-only.

get rid of sync.Map for better go version compatibility#13

Open
zhijinli wants to merge 2 commits into
masterfrom
issue-7
Open

get rid of sync.Map for better go version compatibility#13
zhijinli wants to merge 2 commits into
masterfrom
issue-7

Conversation

@zhijinli
Copy link
Copy Markdown
Collaborator

@zhijinli zhijinli commented Jan 5, 2018

trying to resolve #7 to support Go 1.8

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 5, 2018

Codecov Report

Merging #13 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
+ Coverage   65.97%   66.05%   +0.07%     
==========================================
  Files          20       20              
  Lines        1743     1747       +4     
==========================================
+ Hits         1150     1154       +4     
  Misses        511      511              
  Partials       82       82
Impacted Files Coverage Δ
participant.go 75.48% <100%> (+0.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e5bb1f...383898c. Read the comment docs.

Comment thread participant.go
// stateModelName->stateModelProcessor
stateModelProcessors sync.Map
stateModelProcessorsMu sync.Mutex
stateModelProcessors map[string]*StateModelProcessor // state model name to processor
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: let's avoid inline comment

Comment thread .travis.yml
language: go
go:
- 1.9
- 1.8
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should stick to 1.9 unless there's a huge demand for supporting 1.8.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure about the tradeoff between supporting more versions and losing the 1.9 specific features, but I agree we can punt on this PR and evaluate the tradeoff when demand for 1.8 (or below) really comes.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 10, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get rid of sync.Map

3 participants