-
Notifications
You must be signed in to change notification settings - Fork 170
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
[MISC] Add bep006 and bep010 to completed beps and fix links #186
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This may have some unintended conflicts with the derivatives merge due to the mismatch of this file. I don't want to add more challenges to that PR. Perhaps after that is merged this can be merged? Thank you for putting together this PR nonetheless!
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.
I see what you mean - thanks: I wasn't aware that there are lots of changes in the derivatives PR
https://github.com/bids-standard/bids-specification/pull/109/files#diff-2a8145214cd54ad2715d4543399df795
I guess @chrisfilo can make the call whether we should wait with this or we can go on with it, because he'd be the one who'd have to rebase the derivatives PR.
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.
I would not recommend any PR to wait for #109. I am syncing it with master on a regular basis and will deal with any conflicts.
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.
Given @chrisfilo's comment, could you review/approve once more @franklin-feingold ?
also: @jasmainak perhaps you could give this a quick glance. It's nothing special, but I am adding your name as a moderator of BEP021, so this is your chance to protest or approve :-)
or protestingly approve
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.
you know the perils of asking me to review :-) do we have renewed interest in BEP021? so far I have just been replying to people ... maybe we can look at it once the iEEG/EEG publications go through!
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.
I think we do have interest in BEP021, but I also think we should gather some more experience with EEG and iEEG before moving on with 100% force ;-)
re: your review of this PR: Could you make use of the GitHub tool and approve (or disapprove)? We have community guidlines (and GH settings) here that require that as a formality.
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.
@jasmainak