Skip to content
This repository has been archived by the owner on Sep 1, 2023. It is now read-only.

Replaced NTA_ROOTDIR by NTA #792

Merged
merged 13 commits into from
Apr 22, 2014
Merged

Conversation

david-ragazzi
Copy link
Contributor

$NTA_ROOTDIR is redundant. Its remotion decreases the number of environment variables to the user set.

DavidRagazzi and others added 4 commits April 8, 2014 23:56
This reverts commit 23ba9fa.
$NTA_ROOTDIR is redundant. Its remotion decreases the number of
environment variables to the user set.
@david-ragazzi
Copy link
Contributor Author

Hi @rhyolight . You can merge now..

@rhyolight
Copy link
Member

LGTM, but I'd like @scottpurdy to approve this first.

@scottpurdy
Copy link
Contributor

I don't know what ignore = dirty does for the submodule. And I haven't had a chance to try this. But seems like a move in the right direction.

@david-ragazzi
Copy link
Contributor Author

I don't know what ignore = dirty does for the submodule. And I haven't had a chance to try this. But seems like a move in the right direction.

ignore = dirty option avoids that git include the word -dirty in the end of the submodule SHA when files in a submodule folder are changed.. It's a headache try finding what was changed and revert this changes, as github doesn't give more details about where occurred the changes.. If these changes aren't reverted, it's impossible synchronize nupic repo with a "dirty" submodule..

@breznak
Copy link
Member

breznak commented Apr 10, 2014

On Thu, Apr 10, 2014 at 1:02 PM, David Ragazzi notifications@github.comwrote:

I don't know what ignore = dirty does for the submodule. And I haven't
had a chance to try this. But seems like a move in the right direction.

ignore = dirty option avoids that git include the word -dirty in the end
of the submodule SHA when a submodule folder is changed..

I'm not sure we want this, as git rightfully informs you the something has
changed (?)

It's a headache try finding what was changed and revert this changes, as
github doesn't give more details about where occurred the changes..

git stash will drop the -dirty changes
git diff will tell you what's changed--or that's what I do. (both mus be
done in the nta/ subfolder)

If these changes aren't reverted, it's impossible synchronize nupic repo
with a "dirty" submodule..

@david-ragazzi
Copy link
Contributor Author

Hi @breznak,

As we should pull PRs directly from nupic.core repo cloned to our machine, not from its /nta subfolder in nupic, I think is simpler ignore the changes were in /nta and so avoid type the commands that you suggested..

I saw several developers that recommend this option:

http://stackoverflow.com/questions/3240881/git-can-i-suppress-listing-of-modified-content-dirty-submodule-entries-in-sta

http://www.nils-haldenwang.de/frameworks-and-tools/git/how-to-ignore-changes-in-git-submodules

bronson/vim-update-bundles#32

Anyway, I liked these commands that you suggested, but my lazyness claims for ignore = dirty.. :-)

@david-ragazzi
Copy link
Contributor Author

I reverted "ignore = dirty" in order to this PR is related only to NTA_ROOTDIR env variable.. and I created a specific PR for "ignore = dirty" (#794)..

@rhyolight rhyolight added this to the Sprint 19 milestone Apr 10, 2014
@rhyolight
Copy link
Member

@david-ragazzi Ok, don't hate me for this, but I'm going to wait to merge this PR until Wednesday. Our last merge of PR #751 caused considerable difficulties in the Grok pipelines for @jcasner and the Grok crew, which is my fault for not properly notifying them of the coming build changes. In the meantime (while they mop up the damage and prepare for an upcoming Grok release), they have asked for me to hold of any any PRs that could potentially affect their builds.

This is not a good situation for NuPIC, and it won't last long. Grok engineers will be working next week to improve their pipelines so NuPIC merges that break their pipelines won't prevent them from building and releasing against older versions of NuPIC.

Other PRs that will not potentially affect the Grok builds will still get merged in the meantime.

@david-ragazzi
Copy link
Contributor Author

I'm feel sorry for this @rhyolight . :-(

The problem is I can't imagine the effects of the PRs over Grok and thus it's impossible I advise you about what are the risks involved to merge a PR.. I only run locally, Travis become green, and other people review and test.. This is only what we have to say if something is good or not..

Anyway, I hope this situation normalize ASAP...

@rhyolight
Copy link
Member

I hope this gets resolved soon. We can't expect OS contributors to anticipate how their changes will affect Grok pipelines, so right now the responsibility for that lands squarely on me (how uncomfortable! :hurtrealbad:). So I'm really just covering my own ass here by holding off on this merge.

@jcasner
Copy link

jcasner commented Apr 10, 2014

All, I agree with @rhyolight, this is a temporary scenario because our build logic is flawed internally. We're actively working to resolve that, so this should be the last time we make this request of the NuPIC community. That said, I also have to balance getting commercial software out the door on time 😄

Our builds are all stable at the moment, and I'll check back in here once we have an a final release candidate so that we can merge this one, hopefully sooner than Wednesday.

@unixorn can you please review this one and make sure your team is prepared for the build changes next week?

@rhyolight
Copy link
Member

@jcasner To be clear, I don't think this change will affect Grok builds, but I choose to err on the side of caution.

@david-ragazzi
Copy link
Contributor Author

@rhyolight @jcasner I feel that most PRs are reviewed having in mind what impact these will have on Grok machines.. From I understand, Grok always is compiled using the current state of the Nupic repo.. Hopefully if you tie Grok to a NuPIC (stable) release, the things will be smooth for Grok and OS community, and problems like this will be hard to happen again.. 🙏

@jcasner
Copy link

jcasner commented Apr 10, 2014

@david-ragazzi that's what's supposed to happen. The build logic is currently flawed, so instead of taking the latest stable NuPIC build, we seem to be grabbing the latest build. I think this is actually a recent regression in our pipelines, but at any rate, we're working to correct now...

@rhyolight rhyolight added this to the Sprint 20 milestone Apr 11, 2014
@rhyolight rhyolight removed this from the Sprint 19 milestone Apr 11, 2014
@david-ragazzi
Copy link
Contributor Author

Hi @rhyolight @jcasner @scottpurdy Have you solved Grook issue? Can you merge this?

@rhyolight
Copy link
Member

Hey @david-ragazzi, @jcasner said he would let me know as soon as Grok is released. They are not quite there yet. I'll merge as soon as he tells me.

@jcasner
Copy link

jcasner commented Apr 21, 2014

👍 from Numenta product side now

@rhyolight
Copy link
Member

@david-ragazzi After you resolve the merge conflicts, we are good to merge. 👍

@david-ragazzi
Copy link
Contributor Author

Ok! Ready!

@rhyolight
Copy link
Member

@david-ragazzi Isn't there one more thing? Remove this from the README!

export NTA_ROOTDIR=$NTA

😺

@david-ragazzi
Copy link
Contributor Author

@david-ragazzi Isn't there one more thing? Remove this from the README!
export NTA_ROOTDIR=$NTA

It's weird.. To me it appears as removed... Please, check "file changes" tab again..

@rhyolight
Copy link
Member

😳 uh... too early... need ☕

rhyolight added a commit that referenced this pull request Apr 22, 2014
@rhyolight rhyolight merged commit 7685cc5 into numenta:master Apr 22, 2014
@david-ragazzi
Copy link
Contributor Author

😳 uh... too early... need ☕

hehe

@david-ragazzi david-ragazzi deleted the env-variables branch April 22, 2014 14:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants