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

Improve contributing.md #1488

Merged
merged 4 commits into from
May 28, 2020
Merged

Improve contributing.md #1488

merged 4 commits into from
May 28, 2020

Conversation

st--
Copy link
Member

@st-- st-- commented May 27, 2020

Resolves #1474 and some other minor improvements to contributing.md.

contributing.md Outdated Show resolved Hide resolved
contributing.md Show resolved Hide resolved
contributing.md Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #1488 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1488   +/-   ##
========================================
  Coverage    95.27%   95.27%           
========================================
  Files           82       82           
  Lines         3743     3743           
========================================
  Hits          3566     3566           
  Misses         177      177           

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 d12be33...8483fcc. Read the comment docs.


### I have this big feature/extension I would like to add...

Due to limited scope we may not be able to review and merge every feature, however useful it may be. Particularly large contributions or changes to core code are harder to justify against the scope of the project or future development plans. For such contributions, we suggest you publish them as a separate package that extends GPflow. We can link to your project from an issue discussing the topic or within the repository. Discussing a possible contribution in an issue should give an indication to how broadly it is supported to bring it into the codebase.

### ...but it won't work without changes to GPflow core?

We aim to have the GPflow core infrastructure be sufficiently extensible and modular to enable a wide range of third-party extensions without having to touch the core of GPflow. The `inducing_variables` module is an example of this to enable interdomain approximations (multiscale inducing features, Fourier features, etc.). If your feature/extension does not work outside of GPflow-core because something is hard-coded, please open an issue to discuss this with us!
We aim to have the GPflow core infrastructure be sufficiently extensible and modular to enable a wide range of third-party extensions without having to touch the core of GPflow. The `inducing_variables` module is an example of this to enable interdomain approximations (multiscale inducing features, Fourier features, etc.). If your feature/extension does not work outside of GPflow-core because something is hard-coded, please open an issue to discuss this with us! We are happy to discuss and implement changes to the core code that make it easier for you to extend GPflow with a separate package.
Copy link
Contributor

Choose a reason for hiding this comment

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

We are happy to discuss and implement changes to the core code that make it easier for you to extend GPflow with a separate package.

are we definitely happy to implement the changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

if we discuss and agree we should do it, yes - I don't want it to seem like it'd be up to the external person to fix gpflow-core to make it more modular&extendible.

Copy link
Contributor

Choose a reason for hiding this comment

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

if we discuss and agree we should do it

exactly. but this isn't mentioned in the text (the fact that we'd need to agree to it)

Copy link
Member Author

Choose a reason for hiding this comment

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

Then, please, leave a suggestion as to how you'd word it. I don't see the issue you seem to have with this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: remove "and implement"

@st-- st-- merged commit 3ed381c into develop May 28, 2020
@st-- st-- deleted the st/quickfix/contributing.md branch May 28, 2020 12:26
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.

The code example in contributing.md uses GPflow 1
2 participants