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

Merge max-next-v2016.08-1 into 'next' #974

Closed
wants to merge 12 commits into from

Conversation

eugeneia
Copy link
Member

@eugeneia eugeneia commented Jul 25, 2016

Includes: #969 #971

@eugeneia eugeneia changed the title Merge max-next-v2016.08-1 into 'next Merge max-next-v2016.08-1 into 'next' Jul 25, 2016
@lukego
Copy link
Member

lukego commented Jul 27, 2016

Can we move virtual_ether_mux into e.g. program/snabbnfv/traffic? If it wants to live in lib/ then I think it needs documentation.

@eugeneia
Copy link
Member Author

@lukego Good point, done.

@lukego
Copy link
Member

lukego commented Jul 28, 2016

@eugeneia Looks like the renaming of packetblaster_bench.sh to dpdk_bench.sh is breaking the Hydra test script. The lukego/next-tributary jobs shows the DPDK benchmarks failing on max-next with an error finding the script. Tricky... the new name is better, but renaming successfully requires cross-repo changes i.e. snabblab-nixos test expression needs to be updated to work with both the new and the old name.

On balance I think it makes sense to revert the change of the script name. This keeps the interface compatible for external test scripts (esp. Hydra's nix exp) and should mean that the next-tributary jobset will successfully run the benchmarks to help me avoid accidentally merging a performance regression onto next.

Could you please push a revert of that name change and check that the lukego/next-tributary successfully benchmarks max-next & then resubmit the PR? (Sorry for the inconvenience. Intention is to establish a workflow where Hydra successfully benchmarks the *-next branches before I merge them to next.)

@eugeneia
Copy link
Member Author

@lukego I have added a symlink instead, lets see if that works.

lukego added a commit that referenced this pull request Jul 29, 2016
@lukego
Copy link
Member

lukego commented Jul 29, 2016

@eugeneia That did the trick, thanks!

Going forward could I please pull directly from max-next rather than creating the dedicated branches like max-next-v2016.08-1? This is just because I am now doing extra testing of max-next via my Hydra jobset and that helps me to merge with confidence. The consequence would be that when you have a PR open from max-next you would need to only push commits that you consider ready for merge (since they will be automatically added to the PR).

I actually pulled max-next to get these changes now. I realize now that this brought along #975 before you PR'd that. I hope that is okay with you.

Closing now because the relevant commits have already landed.

@lukego lukego closed this Jul 29, 2016
dpino added a commit to dpino/snabb that referenced this pull request Sep 28, 2017
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.

2 participants